diff --git a/docs/changes/unreleased/1833-ask-here-approvals.md b/docs/changes/unreleased/1833-ask-here-approvals.md new file mode 100644 index 0000000000..91f268f4eb --- /dev/null +++ b/docs/changes/unreleased/1833-ask-here-approvals.md @@ -0,0 +1,13 @@ +--- +kind: fixed +title: ask here answers its approvals in the pane, and its card keeps the question guards +pr: 1833 +surface: [chat, docs] +invalidates: + - "A call inside an `ask here` errand that needed your ok had nowhere to be answered, so the errand's row said `working` until the window closed. The pane now asks it under the call, and `1 allow once` or `3 deny` answers it." + - "asking-from-home.md sent approvals to `continue as a conversation`, which interrupts the waiting call. It now says the approval is answered in the pane, and that continuing ends the waiting call unanswered." + - "Typing a correction after `o` on the pane's card still let `1`, `3` and `0` answer the card, so `make it 10` said yes. Every key is text while the change is written." + - "The pane's card took a key the instant it appeared and filed its answer as a receipt above the window's own box. It now keeps the block's quarter-second settle guard and leaves no receipt there." + - "asking-from-home.md named the card's change key `c`. It is `o`." + - "The pane's stream reader was spelled `errandWait`, so the tui3 test harness treated it as work that always finishes. It is `waitErrand` and is listed in `blockingCommands`." +--- diff --git a/internal/manual/chat/asking-from-home.md b/internal/manual/chat/asking-from-home.md index 50cee1db55..ffe8b083bf 100644 --- a/internal/manual/chat/asking-from-home.md +++ b/internal/manual/chat/asking-from-home.md @@ -35,7 +35,8 @@ Every `ask here` is one row, marked `?`, named with the first line of what you a ``` - **`⠹ working · 4s`** — a turn is in flight. The spinner turns and the clock counts up. -- **`? waiting on you`** — a card is up and nobody has answered it. It is the only tail +- **`? waiting on you`** — a card, or a command's approval, is up and nobody has answered + it. It is the only tail drawn in the bright ink, and it is the same `?` a conversation stopped on a question wears, because it is the same fact: this row costs one keystroke to unblock. - **`∙ stood`** — something now stands because of it. @@ -76,6 +77,10 @@ how the conversation draws a finished call; one that failed carries `✕` and wh **The whole block disappears when the turn ends.** It is a window onto the moment, not a second copy of the transcript — everything in it is already a row above it. +**It also gives way while the turn is stopped on an approval.** Nothing is happening until +you answer, so no spinner turns: the call's row wears the waiting `?` instead, with the +question under it (*Ask here needs your permission to run a command*, below). + A tool row says what the call is pointed at and never says `unknown`: `bash` shows the first line of the command, `read` shows the path, `stand` shows what it is doing (`stand · proposing remind me at 6 to leave`). A call that came back with nothing to say @@ -194,7 +199,8 @@ cursor. **The pane follows the cursor** — walk off the exchange and the row yo draws its own card again, where the frame is wide enough to have one. While the pane has the keyboard the hint reads `enter sends a follow-up · tab or esc back to -the list`, with the card's own answers in front of it when a card is up and +the list`, with the card's own answers in front of it when a card is up — or +`1 allow once · 3 deny` when a command is waiting for your ok — and `↓ continue as a conversation` after it when that row is on screen. **The answers in that line are the ones the card actually drew, and never one more.** A @@ -284,10 +290,11 @@ the pane with your own words, when it would wake, and what it would cost per run created until you answer it: - `1` — yes. It stands as proposed, and the keyboard goes back to the list. -- `c` — change it. The pane says `type the change and press enter`; write the correction in +- `o` — change it. The pane says `type the change and press enter`; write the correction in your own words ("make it 8pm", "every weekday") and the model proposes again. Nothing is - created by a change. (This was `2` before the card's answers moved onto the question every - screen here draws; `c` is that question's own key for "not as it stands".) + created by a change. While you write it every key is text, digits included. (This was `2` + before the card's answers moved onto the question every screen here draws; `o` is that + question's own key for "not as it stands".) - `3`. Once. The action runs now and nothing standing is created. On a repeating check the button reads `Only now, don't repeat`. **Not every card offers it**: a one-off reminder draws no once chip, because doing "remind me at six" now says the @@ -313,9 +320,83 @@ uses: `Set it up · Mondays at 9am · set up`, `Only now, don't repeat · approv ordinary characters again and can be typed into a follow-up. The only card that ever replaces it is the new one the model sends after a change. +## Why didn't my key answer the card in ask here — the quarter-second guard + +**A key pressed within a quarter of a second of the card's answers being drawn is +ignored**, and so is a click. It was aimed at whatever was there before — usually a +follow-up you were typing, or a command's approval you had just answered — so a `1` meant +for something else cannot set something up. Press it again. A card that arrived while you +were in another conversation counts from when you first see it, and one whose answers an +approval hid counts again from when they come back. + The digits belong to a card only while it is still a question. With no card up, or with an answered one on screen, `3` in the middle of "make it 3pm" is just a `3`. +**Your answer is kept on the card itself.** Nothing is added above the message box of the +conversation you open next — it used to show there for half a minute as a decision line, +about a conversation that never asked. + +## I typed a new time and it set up the reminder anyway — typing a change in ask here + +**While you are writing a change after `o`, every key is text.** "make it 10 tomorrow" goes +into the box whole and `enter` sends it; until then neither the card nor a command's +approval beside it takes a digit. Send or clear the sentence, and their keys are theirs +again. + +It used to answer: the `1` was taken as yes, the reminder was set up as first proposed and +the rest of the sentence was lost — and with an approval up beside the card, the `1` +allowed the command. + +## Ask here needs your permission to run a command — approve or deny it in the pane + +When a call inside an `ask here` errand needs your ok — the same approval rules decide it +as in a conversation — the errand stops on it, and **the question is asked in the pane, +directly under the call it is about**: + +``` + ? bash · gh run list --branch main + needs your ok to run bash + bash pattern "gh *" asks first + + ▸ 1 allow once + 3 deny safe answer +``` + +The errand's row on home says `? waiting on you` and sorts to the top. Nothing in the +errand moves until you answer, and **there is no clock on it**: it waits. + +- `1` — allow this one call. It runs, and its row keeps `allowed`. +- `3` — refuse it. The row says `denied` at once, and the model is told + `denied by the person: ` and carries on without it. +- A click anywhere along an answer's row takes that answer. + +Answering leaves the keyboard in the pane, because the errand is still working. + +**The pane decides this one call.** There is no `always` and no `o other` here: a standing +permission is changed with `/permissions`, or answered on a conversation's own approval. +While the question is up, `0` and `2` do nothing, and a letter goes into the follow-up box. + +**A key pressed within a quarter of a second of the question appearing is ignored**, so a +digit you were typing into a follow-up cannot allow a command you have not read. Press it +again. + +**Several calls stopped at once** are asked one at a time. Each shows +`needs your ok to run …` under its own row; the answers appear only under the one the +digits answer, and the next one's once it is next. + +## Why ask here looked stuck on working — an approval nobody could see + +It used to be. When a call inside the errand needed your ok, the pane did not draw the +question at all: the session waited for an answer no screen offered, the row said +`⠹ working` with its clock counting up for as long as the window stayed open, and a `1` +pressed at it went into the follow-up box. The question is now asked in the pane +(*Ask here needs your permission to run a command*, above). + +**If the turn ends while the question is still up** — you quit, or you take the exchange +to `continue as a conversation` — the call does not run, the model is told +`not approved: ended before an answer`, and the question leaves the pane. Continuing as a +conversation does not carry the question across; ask again there if you still want it run. + ## Where did that exchange go — the folder, at every stage There is one folder and it only ever moves. Nothing here copies a transcript and nothing @@ -371,9 +452,10 @@ than moving it a second time. is ever written only into the exchange's own folder, which no screen reads. - **It will not run the full conversation surface in the pane.** The right pane is forty cells wide, so it draws a reduced reading: what you said, what came back **rendered as - markdown**, one line per tool call, the live strip, and the card. Slash commands, the task - column, `/rewind`, images and the approval card are all the conversation's own screen — - take the exchange there with `continue as a conversation` if you need them. + markdown**, one line per tool call, the live strip, the card, and a command's approval + with its two answers. Slash commands, the task column, `/rewind`, images and an approval's + `always` are all the conversation's own screen — take the exchange there with + `continue as a conversation` if you need them. - **It will not create anything without the card.** Typing a sentence at home does not arm a reminder; a card does, and only after you press `1`. - **It will not end an errand because you looked away.** Closing home, opening another diff --git a/internal/manual/chat_test.go b/internal/manual/chat_test.go index 04b2cdcb9a..52023ce6f5 100644 --- a/internal/manual/chat_test.go +++ b/internal/manual/chat_test.go @@ -2530,6 +2530,18 @@ func TestTheChatManualAnswersTheQuestionsPeopleAsk(t *testing.T) { {"why is the answer from home showing asterisks and hashes", "asking-from-home"}, {"does the ask here pane format the answer", "asking-from-home"}, {"can I ask two things from home at once", "asking-from-home"}, + // A call inside an `ask here` errand that needed the person's ok had + // nowhere to be answered: the row said `working` for as long as the + // window was open. These are what somebody types looking at that row, + // and at the question the pane now draws instead. + {"ask here is stuck on working", "asking-from-home"}, + {"a command in ask here needs my permission", "asking-from-home"}, + {"how do I approve a command in the ask here pane", "asking-from-home"}, + // The pane's card owned its digits while a correction was being typed, so + // "make it 10" set the reminder up at the `1`; and it took a key pressed + // the instant it appeared. What somebody asks after either. + {"I typed a new time and it set up the reminder anyway", "asking-from-home"}, + {"why didn't my key answer the card in ask here", "asking-from-home"}, {"why did it set my reminder for a time that already passed", "keeping-an-eye"}, {"why is there no once on my reminder card", "keeping-an-eye"}, // The e2e suite found the firing that reached a conversation and was diff --git a/internal/tui3/harnessdriver_test.go b/internal/tui3/harnessdriver_test.go index f474b47fcd..e1e182ae19 100644 --- a/internal/tui3/harnessdriver_test.go +++ b/internal/tui3/harnessdriver_test.go @@ -777,6 +777,7 @@ func TestTheHarnessOverlapsEveryWaiterItNames(t *testing.T) { // table without a case here fails this test rather than slipping through. built := map[string]tea.Cmd{ "waitDesign": waitDesign(events, 1), + "waitErrand": waitErrand(nil, events), "waitEvent": waitEvent(events, 1), "waitGuestNotices": waitGuestNotices(events, 1), "waitGuestQuestions": waitGuestQuestions(events, 1), diff --git a/internal/tui3/homeexchange.go b/internal/tui3/homeexchange.go index 479bbc174c..e9b826adbc 100644 --- a/internal/tui3/homeexchange.go +++ b/internal/tui3/homeexchange.go @@ -238,6 +238,35 @@ type exchangeRow struct { // object [homeExchange.view] holds while it is the current one, so the row // and the keyboard can never disagree about what was decided. card *standingCard + // callID is the provider's id for the call a tool row draws, and it is how + // the call's result and the approval the gate raises about it find THIS row. + // The engine sends it on every begin, end and consent request because one + // step can call one tool several times, and a pairing by verb would let a + // person read one command and allow another ([session.Event.CallID]). + callID string + // decided is what the person answered the approval about this call — + // `allowed` or `denied`, the conversation's own words for the same answer + // (consent.go's [decisionWord]) — and "" for a call nobody was asked about. + // The row keeps it after the call comes back: "you were asked and said yes" + // is part of what happened, and the transcript records it nowhere else. + decided string +} + +// exchangePermit is one approval the errand's gate is holding a call for: the +// question as the block's renderer reads it, and the row of the call it is +// about. +// +// IT IS THE PANE'S OWN AND NEVER THE BLOCK'S, for [homeExchange.ask]'s reason +// and one more. The question is about another session, so it cannot wait above +// the window's message box; and the block's door ([app.answerQuestion]) files a +// receipt above THAT box whose `c change` is answered by the window's own +// engine, which would be re-asking an errand's decision of a session that +// never asked it. So the answer goes straight to the errand's agent +// ([app.answerPermit]) and leaves its record on the call's row instead. +type exchangePermit struct { + shown questionShown + // call is the [exchangeRow.callID] it hangs under. + call string } // homeExchange is one errand: the agent, the folder it writes into, what has @@ -330,6 +359,21 @@ type homeExchange struct { // changing arms the correction: `2` on the card says the next enter is not // a follow-up but the person's own wording of what is wrong with it. changing bool + // permits is every approval this errand's turn is stopped on, oldest first. + // + // THE PANE IS THE ONLY PLACE THEY CAN BE ANSWERED. The errand's session is + // built with the gate allowed to ask — its own card needs that door — so a + // call the policy grades PROMPT blocks inside it until this agent is told + // yes or no (internal/session's consent.go), and nothing else in the program + // holds this agent: home's answer band reads presence files under the + // projects root, and an errand's folder is not under it. Before this list the + // fold let the request fall through, and the errand sat `working` on a + // question no screen drew for as long as the window was open. + // + // IT IS A LIST because one step can stop on several calls at once. The digits + // answer the oldest ([homeExchange.permit]); the rest wait their turn under + // their own rows. + permits []*exchangePermit // stood is set once something actually stands. It is what stops a second // move of one folder, and what the pane says about where the record went. @@ -377,14 +421,15 @@ type homeExchange struct { // turn in flight — and a fourth copy of a fact is a fourth thing to keep in // step. -// waiting reports whether a card is up and unanswered. It is the state that -// sorts first everywhere on this screen. -func (ex *homeExchange) waiting() bool { return ex.asking() } +// waiting reports whether a question is up and unanswered: the card, or an +// approval the turn is stopped on. It is the state that sorts first everywhere +// on this screen. +func (ex *homeExchange) waiting() bool { return ex.asking() || len(ex.permits) > 0 } // over reports whether the exchange has nothing left to do: the card, if there -// was one, has been answered — stood, once, or declined — and the agent is -// idle. -func (ex *homeExchange) over() bool { return !ex.working && !ex.asking() } +// was one, has been answered — stood, once, or declined — no approval is +// waiting, and the agent is idle. +func (ex *homeExchange) over() bool { return !ex.working && !ex.waiting() } // spent reports whether the exchange may be filed: it is over AND the person // has seen it that way. [app.sweepExchanges] adds the third condition, which is @@ -489,15 +534,16 @@ func (a *app) errandUpdate(msg errandMsg) tea.Cmd { // caller got a closed channel, exactly as session.Agent documents. return nil } - return errandWait(ex, m.ch) + return waitErrand(ex, m.ch) case errandEventMsg: // The fold first, then the next read. Both, always: an event that told // the pane nothing still has to be followed by the one that does. cmd := a.errandEvent(ex, m.ev) - return tea.Batch(cmd, errandWait(ex, m.ch)) + return tea.Batch(cmd, waitErrand(ex, m.ch)) case errandClosedMsg: ex.working = false ex.closeReply() + ex.dropPermits() } return nil } @@ -528,10 +574,16 @@ func (a *app) holdsExchange(ex *homeExchange) bool { return false } -// errandWait is [waitEvent] for this lane. It carries the exchange itself +// waitErrand is [waitEvent] for this lane. It carries the exchange itself // rather than a generation number, because an exchange IS its own generation: // it is a pointer nothing else can be, and several of them are open at once. -func errandWait(ex *homeExchange, ch <-chan session.Event) tea.Cmd { +// +// IT IS SPELLED LIKE EVERY OTHER WAITER ON PURPOSE. It parks for as long as the +// errand's turn is held open — which a gated call does until somebody answers +// it — so it is a command that may never answer, and the harness finds those by +// their `wait` prefix (tui3_test.go's blockingCommands). Spelled errandWait, it +// was billed as work that always finishes. +func waitErrand(ex *homeExchange, ch <-chan session.Event) tea.Cmd { return func() tea.Msg { ev, ok := <-ch if !ok { @@ -541,14 +593,19 @@ func errandWait(ex *homeExchange, ch <-chan session.Event) tea.Cmd { } } -// errandEvent is the reduced fold: text, tools, the card, the update that says -// something now stands, and nothing else. +// errandEvent is the reduced fold: text, tools, the approval a call is stopped +// on, the card, the update that says something now stands, and nothing else. // // IT IGNORES MOST OF WHAT A TURN EMITS, and that is the design rather than a // gap. Reasoning, compaction, harness offers, task proposals and the rest are // all things the conversation's own surface draws with rows, clocks and answer // lanes it has the width for; an errand is two sentences and a card, and a pane // that tried to be a second transcript in forty cells would be neither. +// +// WHAT IT MAY NOT IGNORE IS A QUESTION THE TURN IS BLOCKED ON. Everything above +// can be left out because the turn goes on without it; an approval is the one +// event after which nothing happens until somebody answers, and this pane is +// the only screen that holds the agent to answer it ([homeExchange.permits]). func (a *app) errandEvent(ex *homeExchange, ev session.Event) tea.Cmd { switch ev.Kind { case session.EventTextDelta: @@ -564,11 +621,15 @@ func (a *app) errandEvent(ex *homeExchange, ev session.Event) tea.Cmd { ex.closeReply() ex.rows = append(ex.rows, exchangeRow{ kind: exchangeTool, text: exchangeToolLine(ev), began: a.now(), + callID: strings.TrimSpace(ev.CallID), }) case session.EventToolEnd, session.EventToolFailed: a.closeExchangeTool(ex, ev) + case session.EventConsentRequest: + a.exchangeConsent(ex, ev) + case session.EventStandingProposal: if ev.Standing != nil { a.exchangeProposal(ex, *ev.Standing) @@ -586,10 +647,208 @@ func (a *app) errandEvent(ex *homeExchange, ev session.Event) tea.Cmd { case session.EventTurnDone: ex.closeReply() ex.working = false + ex.dropPermits() } return nil } +// ── an approval the errand's turn is stopped on ───────────────────────────── + +// exchangeConsent puts one approval into the pane, directly under the call it +// is about. +// +// THE CALL IS DRAWN WHERE IT IS NOT ALREADY, for consent.go's own reason: a +// question about a call nobody can see is a question nobody can answer. The +// engine announces the call before the gate stops it, so the row is normally +// there and is found by its id; a request with no row to hang under draws one +// from its own arguments, which are the arguments the gate is asking about. +// +// AND A REQUEST SENT AGAIN REPLACES ITSELF. The id is the gate's, and one id is +// one question however many times it arrives. +func (a *app) exchangeConsent(ex *homeExchange, ev session.Event) { + ex.closeReply() + call := strings.TrimSpace(ev.CallID) + if call == "" { + // NO ID TO PAIR BY IS NOT A REASON TO GUESS. A row chosen by verb could + // be another call of the same tool, so the request gets a row of its own, + // named by the gate's id so nothing else can claim it. + call = "consent:" + itoa(int(ev.ID)) + } + if ex.callRow(call) == nil { + ex.rows = append(ex.rows, exchangeRow{ + kind: exchangeTool, text: exchangeToolLine(ev), began: a.now(), callID: call, + }) + } + // AND THE CARD'S ANSWERS GO, SO ITS SETTLE GUARD STARTS AGAIN. They are not + // drawn while an approval is up, and when they come back they come back under + // a hand that was just answering the approval — `1 1` must allow the call and + // leave the card alone. The next frame that draws them stamps them afresh. + if ex.ask != nil { + ex.ask.shown = time.Time{} + } + permit := &exchangePermit{shown: a.exchangePermitShown(ex, ev), call: call} + for i, open := range ex.permits { + if open.shown.question.ID == ev.ID { + ex.permits[i] = permit + return + } + } + ex.permits = append(ex.permits, permit) +} + +// exchangePermitShown is one request as the question the block's renderer +// draws, built by the conversation's own builder ([app.consentQuestion]) so the +// sentence, the reason and the answers are the ones a person reads about the +// same call anywhere else. +// +// TWO THINGS ARE SAID DIFFERENTLY HERE, and both are about where it is drawn: +// +// - THERE IS NO `always`. The widening answer writes a standing permission — +// a rule in the person's own settings, or a memo for the rest of the +// session — and from this pane it would be written beside the window's +// conversation rather than the errand's, or last only as long as an errand +// does. A standing yes is the policy's to change (/permissions); what is +// decided here is this one call, which is what `allow once` and `deny` are. +// - IT POINTS AT NO ROW OF THE WINDOW'S. The token carries the folder, as the +// card's does ([app.exchangeShown]), and the subject is left empty: the call +// is a row of this pane, and a subject the block resolved against the +// window's own transcript could name a different call with the same id. +func (a *app) exchangePermitShown(ex *homeExchange, ev session.Event) questionShown { + q := a.consentQuestion(ev) + kept := make([]session.AnswerOption, 0, len(q.Options)) + for _, option := range q.Options { + if !option.Widening { + kept = append(kept, option) + } + } + q.Options = kept + q.Scope = []session.AnswerScope{session.ScopeOnce} + q.Ref = ex.id + "/" + itoa(int(ev.ID)) + q.Subject = session.SubjectRef{} + q.Asked = a.now() + return questionShown{question: q, pick: questionPointerStart(q)} +} + +// permit is the approval the pane's digits answer: the oldest one waiting, and +// nil when none is. +func (ex *homeExchange) permit() *exchangePermit { + if len(ex.permits) == 0 { + return nil + } + return ex.permits[0] +} + +// permitOn is the approval waiting on one row's call, and nil for a row whose +// call nobody is being asked about. +func (ex *homeExchange) permitOn(row exchangeRow) *exchangePermit { + if row.kind != exchangeTool || row.callID == "" { + return nil + } + for _, permit := range ex.permits { + if permit.call == row.callID { + return permit + } + } + return nil +} + +// callRow is the tool row one call drew, and nil when none did. +func (ex *homeExchange) callRow(call string) *exchangeRow { + if call == "" { + return nil + } + for i := len(ex.rows) - 1; i >= 0; i-- { + if ex.rows[i].kind == exchangeTool && ex.rows[i].callID == call { + return &ex.rows[i] + } + } + return nil +} + +// dropPermits forgets every approval still waiting. It runs when the turn that +// raised them ends — finished, interrupted, or its stream closed — because the +// session released those calls when its context died: the model has already +// been told `not approved: ended before an answer`, and an answer now would be +// late for work that is over (consent.go's [app.dropAsks] is the same rule). +func (ex *homeExchange) dropPermits() { ex.permits = nil } + +// exchangePermitKey is a key pressed in the pane while an approval is up, and +// whether the approval took it. +// +// THE APPROVAL'S OWN ANSWERS ARE TAKEN, AND NO OTHER DIGIT IS TYPED. A digit +// with no answer under it is inert, which is the card's law in this pane: a +// stray `2` left in the box would make the next enter a follow-up nobody meant. +// A LETTER IS NOT TAKEN — `o` asks the card for the box, and an approval has +// no such answer here — so it goes to the box the person is typing in. +// +// AND THE SETTLE GUARD IS KEPT, as it is on the card. A key that arrives before +// the question has been on the screen for [questionSettle] was aimed at whatever +// was there before it — the follow-up box, or the approval just answered when two +// are queued — and the one answer here that must never come from a keystroke +// meant for something else is `allow once`. +func (a *app) exchangePermitKey(ex *homeExchange, key string) (tea.Cmd, bool) { + permit := ex.permit() + if permit == nil { + return nil, false + } + if _, ok := permit.shown.question.Option(key); !ok { + return nil, key != questionCommentKey + } + if !a.questionSettled(permit.shown) { + return nil, true + } + return a.answerPermit(ex, permit, key), true +} + +// pickPermit is a press on one of the approval's answers, by its index on the +// card, under the same guard as the key that names it. +func (a *app) pickPermit(ex *homeExchange, at int) tea.Cmd { + permit := ex.permit() + if permit == nil || at < 0 || at >= len(permit.shown.question.Options) { + return nil + } + if !a.questionSettled(permit.shown) { + return nil + } + return a.answerPermit(ex, permit, strings.TrimSpace(permit.shown.question.Options[at].Key)) +} + +// answerPermit gives one approval its answer, on the errand's own session. +// +// THE ANSWER IS THE ENGINE'S OWN MAPPING ([session.AnswerFromKey]) and it is +// sent as exactly what this pane offered: this call, once. The row of the call +// keeps what was decided — an allowed call's clock starts again from the answer, +// because the wait for a person was never the call's runtime, and a refused call +// is closed at once rather than spinning until the rest of its step comes back. +func (a *app) answerPermit(ex *homeExchange, permit *exchangePermit, key string) tea.Cmd { + action, ok := session.AnswerFromKey(session.QuestionConsent, key) + if !ok { + return nil + } + for i, open := range ex.permits { + if open == permit { + ex.permits = append(ex.permits[:i], ex.permits[i+1:]...) + break + } + } + if row := ex.callRow(permit.call); row != nil { + row.decided = decisionWord(action.Allow) + if action.Allow { + row.began = a.now() + } else { + row.done, row.failed = true, true + } + } + id, allow, agent := permit.shown.question.ID, action.Allow, ex.agent + // FROM A COMMAND, NEVER FROM THE LOOP (offloop.go), and through the ordered + // line, because two approvals answered in a row must reach the engine in the + // order they were given. + return a.offLoop(func() func(bool) tea.Cmd { + agent.ResolveConsent(id, allow) + return nil + }) +} + // exchangeProposal puts one ratification card into the pane, in the transcript // where it arrived. // @@ -645,13 +904,15 @@ func (a *app) exchangeShown(ex *homeExchange, notice session.StandingNotice) *qu return a.answerExchangeCard(ex, &kept, answer) }, commented: func() { ex.changing = true }, - } - // THE STAMP IS WRITTEN AT THE RAISE, which is [app.raiseHomeAsk]'s own - // bargain: the settle guard exists to catch a key aimed at the screen a - // quarter-second ago, and the pane redraws this card on every frame it is on - // — so a stamp waiting for the block's own draw would never be written at - // all, and the digits would never answer. - shown.shown = a.now() + // AND ITS RECEIPT IS THE CARD ITSELF, settled in place. The block's + // receipt is drawn above the WINDOW's box, which holds a different + // conversation from the one this card is about ([questionShown.elsewhere]). + elsewhere: true, + } + // THE STAMP IS NOT WRITTEN HERE. The settle guard is a claim about what a + // person could have seen, so it counts from the first frame that DREW the + // answers ([app.exchangePane]) — a card that arrived while somebody was in + // another conversation has been on nobody's screen, however long ago it came. return &shown } @@ -727,9 +988,10 @@ func (ex *homeExchange) settle(verdict, answer string) { ex.ask, ex.changing = nil, false } -// closeExchangeTool puts a call's result on the row that opened it — the newest row of -// that verb with nothing back yet — and opens a fresh row when there is none, -// so a result can never land on the wrong call. +// closeExchangeTool puts a call's result on the row that opened it — the row +// with the call's own id, or the newest row with nothing back yet where the +// engine sent none — and opens a fresh row when there is none, so a result can +// never land on the wrong call. // // AND NOTHING HERE EVER SAYS "unknown". It used to reach for [errText] as the // fallback gloss, and [errText] answers "unknown" for a nil error — so every @@ -745,6 +1007,22 @@ func (a *app) closeExchangeTool(ex *homeExchange, ev session.Event) { if failed && ev.Err != nil { hint = strings.TrimSpace(ev.Err.Error()) } + // THE CALL'S OWN ID FIRST, where the engine sent one: a step that ran two + // calls ends them in call order, not in the order they finished, so the + // newest open row is a guess the id makes unnecessary. + if row := ex.callRow(strings.TrimSpace(ev.CallID)); row != nil { + if row.done { + // A CALL REFUSED IN THIS PANE IS ALREADY CLOSED, and what the engine + // sends after it is its own account of that refusal — the sentence + // the MODEL reads. The row keeps the person's word ([answerPermit]). + return + } + row.done, row.failed, row.hint = true, failed, hint + if !row.began.IsZero() { + row.took = a.now().Sub(row.began) + } + return + } for i := len(ex.rows) - 1; i >= 0; i-- { row := &ex.rows[i] if row.kind != exchangeTool || row.done { @@ -758,7 +1036,7 @@ func (a *app) closeExchangeTool(ex *homeExchange, ev session.Event) { } ex.rows = append(ex.rows, exchangeRow{ kind: exchangeTool, text: exchangeToolLine(ev), hint: hint, - done: true, failed: failed, + done: true, failed: failed, callID: strings.TrimSpace(ev.CallID), }) } @@ -1310,10 +1588,34 @@ func (a *app) exchangeKey(ex *homeExchange, msg tea.KeyPressMsg) tea.Cmd { // hands the keyboard back to the list, and a card left standing on the // column is not an answer. // - // AND `c` ARMS THE CORRECTION, which is what `2 change when or where` + // AND `o` ARMS THE CORRECTION, which is what `2 change when or where` // used to be — a chip that asked for the box rather than an answer. It is // the same key on every question this surface asks. - if ex.asking() && ex.ask != nil { + // + // WHILE A CORRECTION IS BEING WRITTEN THE BOX OWNS EVERY KEY, and that + // comes before either question: `make it 10 tomorrow` is a sentence. Its + // `1` used to answer the card yes halfway through the word meant to + // change it — and with an approval up beside the card, it allowed the + // call. Both questions wait until the sentence is sent or cleared. + // + // AN APPROVAL THE TURN IS STOPPED ON GOES NEXT. Nothing in the errand + // moves until it is answered, the card's answers are not drawn while it + // is up, and a key may only answer what is on the screen. + if ex.changing { + break + } + if ex.permit() != nil { + if cmd, took := a.exchangePermitKey(ex, msg.String()); took { + return cmd + } + } else if ex.asking() && ex.ask != nil { + // THE SETTLE GUARD HOLDS HERE AS IT DOES ON THE BLOCK. A key that + // arrives before the card has been on the screen for [questionSettle] + // was aimed at what was there before it — a `1` in a follow-up — and is + // dropped rather than read as `set it up` ([app.questionSettled]). + if !a.questionSettled(*ex.ask) { + return nil + } if cmd, took := a.questionOptionKey(*ex.ask, msg.String()); took { return cmd } @@ -1599,34 +1901,41 @@ func (a *app) exchangePane(ex *homeExchange, width, room int, pal palette) []str continue } out = append(out, StandingCardRows(a, row.card, width, true)...) - if row.card == ex.view && ex.ask != nil { + if row.card == ex.view && ex.ask != nil && ex.permit() == nil { // AND THE QUESTION DIRECTLY UNDER THE CARD IT IS ABOUT, drawn by // the block's own renderer. It is not pinned above a message box // here because there is no message box in this pane that belongs // to this errand's conversation — the card and its answers are one // thing on the column, and separating them would put a decision on // a different part of the screen from the thing being decided. - at := len(out) - out = append(out, a.questionPanelBody(*ex.ask, width)...) - ex.askAt = make([]int, len(ex.ask.question.Options)) - for i := range ex.askAt { - ex.askAt[i] = -1 - } - for _, band := range a.questionBands { - if band.at >= 0 && band.at < len(ex.askAt) { - ex.askAt[band.at] = at + band.row - } + // + // NOT WHILE AN APPROVAL IS UP: the digits answer that first + // ([app.exchangeKey]), and one digit drawn under two questions is a + // key whose meaning depends on which one a person happened to read. + if ex.ask.shown.IsZero() { + ex.ask.shown = a.now() } + answers, askAt := a.exchangeAnswers(*ex.ask, width, len(out)) + out, ex.askAt = append(out, answers...), askAt } out = separated(out) continue } + if permit := ex.permitOn(row); permit != nil { + out = append(out, a.exchangePermitLines(ex, permit, row, len(out), width, pal)...) + continue + } out = append(out, a.exchangeRowLines(row, width, pal)...) } if ex.changing { out = append(out, pal.accent(fit(homeAskChangeWord, width))) } - if ex.working { + // THE LIVE BLOCK GIVES WAY TO AN APPROVAL. It says what is happening this + // second, and while the turn is stopped on a person nothing is: a spinner + // there would be the pane claiming work it is in fact waiting to be allowed + // to do, which is the reading a call parked on a question must never have + // (consent.go's [app.askConsent]). The approval itself is the live state. + if ex.working && ex.permit() == nil { out = append(out, a.exchangeLive(ex, width, pal)...) } if ex.offering() { @@ -1656,6 +1965,74 @@ func (a *app) exchangePane(ex *homeExchange, width, room int, pal palette) []str return out } +// exchangeAnswers draws one question's answers with the block's own renderer +// ([app.questionPanelBody]) and answers where each landed, as pane rows +// counted from at — -1 for an answer that drew no row. +// +// IT READS ONLY THE BANDS IT WROTE, AND TAKES THEM BACK OFF. [app.questionBands] +// is the block's press map, and rows this pane left in it would be a click in +// the conversation resolved against an errand's answers — so they are read and +// removed in one place, which is the panel's own bargain when it lays a body it +// will not keep (questionpanel.go). +func (a *app) exchangeAnswers(q questionShown, width, at int) ([]string, []int) { + first := len(a.questionBands) + rows := a.questionPanelBody(q, width) + askAt := make([]int, len(q.question.Options)) + for i := range askAt { + askAt[i] = -1 + } + for _, band := range a.questionBands[first:] { + if band.at >= 0 && band.at < len(askAt) { + askAt[band.at] = at + band.row + } + } + a.questionBands = a.questionBands[:first] + return rows, askAt +} + +// exchangePermitLines draws one call the gate is holding, where its row was: the +// call marked as waiting on a person, the gate's own sentence under it, and — on +// the approval the digits answer — the answers, drawn by the block's renderer. +// +// ? bash · gh run list --branch main +// needs your ok to run bash +// bash pattern "gh *" asks first +// +// ▸ 1 allow once +// 3 deny safe answer +// +// THE ROW STOPS CLAIMING TO BE WORKING. Its spinner and its clock are the +// conversation's marks for a call that is executing, and this one is waiting +// for somebody; the mark is the one the conversation gives the same call +// (toolview.go's [app.mark] for a call blocked on a person), through the +// vocabulary's one door. +// +// A LATER APPROVAL WAITS ITS TURN WITHOUT ITS ANSWERS. The digits answer the +// oldest, so the same `1` drawn under two calls would be a key whose meaning +// depends on which one a person read; the second shows its sentence, and its +// answers when it is next. +// +// AND THE SETTLE GUARD COUNTS FROM HERE. The stamp is written the first time the +// answers are DRAWN, because the guard is a claim about what a person could have +// seen ([app.markQuestionShown] says the same about the block) — an approval +// that arrived while the cursor was on another row has been on nobody's screen. +func (a *app) exchangePermitLines(ex *homeExchange, permit *exchangePermit, row exchangeRow, at, width int, pal palette) []string { + mark := pal.askBold(a.icon(tokens.GNeedsHuman)) + out := []string{mark + " " + pal.dim(fit(row.text, width-2))} + for _, line := range wrap(strings.TrimSpace(permit.shown.question.Head), max(width-len(questionPanelGap), 1)) { + out = append(out, questionPanelGap+pal.ink(line)) + } + if permit != ex.permit() { + return append(out, "") + } + if permit.shown.shown.IsZero() { + permit.shown.shown = a.now() + } + answers, askAt := a.exchangeAnswers(permit.shown, width, at+len(out)) + ex.askAt = askAt + return append(out, answers...) +} + // exchangeRowLines draws one row of the exchange at a width. // // THE THREE SHAPES ARE THE TRANSCRIPT'S THREE, reduced but never renamed: what @@ -1714,7 +2091,17 @@ func (a *app) exchangeRowLines(row exchangeRow, width int, pal palette) []string out = append(out, pal.ink(wrapped)) } case exchangeTool: - out = append(out, a.exchangeToolLine(row, row.hint, width, pal)) + // WHAT THE PERSON ANSWERED ABOUT THE CALL COMES BEFORE WHAT CAME BACK, + // because it happened first: `allowed · exit status 1` is a yes followed + // by a failure, and the other order would read as the reason for it. + words := row.hint + if row.decided != "" { + words = row.decided + if hint := strings.TrimSpace(row.hint); hint != "" { + words += " · " + hint + } + } + out = append(out, a.exchangeToolLine(row, words, width, pal)) case exchangeNote: for _, wrapped := range wrap(row.text, width) { out = append(out, pal.dim(wrapped)) @@ -2100,7 +2487,18 @@ func (a *app) exchangePress(x, row int) tea.Cmd { ex.onOffer = true return a.promoteExchange(ex) } - if ex.asking() && ex.ask != nil { + if ex.permit() != nil { + // THE ANSWERS ON SCREEN ARE THE APPROVAL'S while one is up ([app. + // exchangePermitLines] wrote these targets), and a press on one goes the + // way its digit does. + for at, drawn := range ex.askAt { + if drawn >= 0 && drawn == row { + return a.pickPermit(ex, at) + } + } + } else if ex.asking() && ex.ask != nil && a.questionSettled(*ex.ask) { + // A press is held to the settle guard for the reason its digit is: a + // click aimed at a row the card has just moved under is not an answer. for at, drawn := range ex.askAt { if drawn < 0 || drawn != row { continue @@ -2167,7 +2565,13 @@ func exchangeHint(ex *homeExchange) string { return homeAskChangeWord + " · esc clear" } var parts []string - if ex.asking() { + if permit := ex.permit(); permit != nil { + // AN APPROVAL'S ANSWERS ARE ITS TWO AND NOTHING ELSE. It has no `o` + // here — a sentence in this box is a follow-up, not a reason beside the + // call — and no `always` ([app.exchangePermitShown] says why), so the + // line names exactly the digits [app.exchangePermitKey] takes. + parts = append(parts, exchangeAnswerWords(permit.shown.question)) + } else if ex.asking() { // THE ANSWERS THE CARD DREW, off the question that drew them // ([app.questionHintOn]). This clause used to be a hardcoded copy of a // line kept correctly elsewhere, and it named `3 just once` over a diff --git a/internal/tui3/homeexchange_cardguard_test.go b/internal/tui3/homeexchange_cardguard_test.go new file mode 100644 index 0000000000..2092bd5673 --- /dev/null +++ b/internal/tui3/homeexchange_cardguard_test.go @@ -0,0 +1,200 @@ +package tui3 + +import ( + "strings" + "testing" + "time" + + "github.com/Agent-Field/codeaf/internal/session" +) + +// ── the pane's card, held to the block's own laws ──────────────────────────── +// +// Home's `ask here` pane routes its card's keys straight into the block's router +// ([app.questionOptionKey]), past the two guards the block keeps in front of it +// (question.go's FOUR LAWS): a key that lands before the question has been on the +// screen is dropped, and a box somebody is writing in is never answered out from +// under them. These tests are those two laws in the pane, and the third thing the +// block's door did there that it should not: file the card's answer as a receipt +// above the window's own conversation. + +// askCard opens home on a frame wide enough to draw the pane beside the list, +// pins the clock, and asks one thing whose turn proposes a reminder. The card is +// up and has NOT been drawn yet. The clock is handed back so a test can move it. +func askCard(t *testing.T) (*app, *errandLab, *homeExchange, *time.Time) { + t.Helper() + lab := newErrandLab(t) + mine := lab.session("-tmp-alpha", "aaaa000000000001", "pricing research", "/tmp/alpha", time.Now()) + a := lab.app(mine, []session.Event{ + standingProposal(7, "remind me at 6 to leave"), + {Kind: session.EventTurnDone}, + }) + besideTheList(a) + now := time.Now() + a.clock = func() time.Time { return now } + a.openHome() + typeHome(a, "remind me at 6 to leave") + drive(t, a, key("up"), key("enter")) + ex := theExchange(a) + if ex == nil || !ex.asking() { + t.Fatal("the turn did not leave a card asking in the pane") + } + return a, lab, ex, &now +} + +// TestTypingACorrectionNeverAnswersTheCard is the defect worth the most of the +// three. After `o` the pane says `type the change and press enter`, and the +// sentence a person types is a time — so it is full of digits. The card went on +// owning `1`, `3` and `0` while it waited for that sentence: `make it 10 +// tomorrow` answered the card yes at its `1`, set up the reminder as first +// proposed, and dropped the rest of the words. +func TestTypingACorrectionNeverAnswersTheCard(t *testing.T) { + a, lab, ex, now := askCard(t) + homeText(a) + *now = now.Add(time.Second) + + drive(t, a, key(questionCommentKey)) + if !ex.changing { + t.Fatalf("`%s` did not arm the correction", questionCommentKey) + } + const words = "make it 10 tomorrow, or 3 if that is too late" + typeHome(a, words) + if len(lab.agent.answered) != 0 { + t.Fatalf("typing the correction answered the card: %+v", lab.agent.answered) + } + if got := ex.box.String(); got != words { + t.Fatalf("the correction lost characters to the card, the box holds %q", got) + } + if !ex.asking() || !ex.changing { + t.Fatal("the card stopped waiting for its correction while it was being typed") + } + drive(t, a, key("enter")) + if len(lab.agent.answered) != 1 || lab.agent.answered[0].Change != words || lab.agent.answered[0].Approved { + t.Fatalf("enter did not send the whole correction, the agent saw %+v", lab.agent.answered) + } +} + +// TestADigitThatLandsWithTheCardIsDropped is the settle guard: a card that has +// not been drawn, or was drawn less than [questionSettle] ago, takes no key and +// no press — they were aimed at whatever was on the screen before it — and once +// it has been read, the same key answers it. +func TestADigitThatLandsWithTheCardIsDropped(t *testing.T) { + a, lab, ex, now := askCard(t) + + // NOT YET DRAWN: the card has been on nobody's screen. + drive(t, a, key("1")) + if len(lab.agent.answered) != 0 { + t.Fatalf("a key answered a card nobody had seen: %+v", lab.agent.answered) + } + // DRAWN THIS INSTANT: a key and a press both land with it. + homeText(a) + drive(t, a, key("1")) + if at := ex.askAt; len(at) > 0 && at[0] >= 0 { + spend(t, a, a.exchangePress(0, at[0])) + } + if len(lab.agent.answered) != 0 { + t.Fatalf("a key or press on the heels of the card answered it: %+v", lab.agent.answered) + } + if got := ex.box.String(); got != "" { + t.Fatalf("a dropped key was typed into the box as %q", got) + } + if !ex.asking() { + t.Fatal("a dropped key took the card down") + } + // READ: the same key is now an answer. + *now = now.Add(questionSettle) + drive(t, a, key("1")) + if len(lab.agent.answered) != 1 || !lab.agent.answered[0].Approved { + t.Fatalf("a settled `1` did not answer the card, the agent saw %+v", lab.agent.answered) + } +} + +// TestTheCardsAnswerLeavesNoReceiptInTheWindow is the card's answer kept where +// it belongs. The block's door files a receipt above the WINDOW's message box, +// and the window is holding a different conversation from the errand — so the +// answer to a reminder asked from home turned up as a decision line in whatever +// conversation was opened next. The card settles in place and is its own record. +func TestTheCardsAnswerLeavesNoReceiptInTheWindow(t *testing.T) { + a, lab, ex, _ := askCard(t) + readCard(t, a) + + drive(t, a, key("1")) + if len(lab.agent.answered) != 1 { + t.Fatalf("`1` did not answer the card, the agent saw %+v", lab.agent.answered) + } + if len(a.questionRecords) != 0 { + t.Fatalf("the window filed a receipt for the errand's card: %+v", a.questionRecords) + } + if !ex.view.settled() || !strings.Contains(homeText(a), "remind me at 6 to leave") { + t.Fatalf("the card did not keep its answer in the pane:\n%s", homeText(a)) + } +} + +// TestACorrectionNeverAnswersAnApprovalBesideTheCard is the same law with both +// questions up. One step can propose a reminder and stop on a gated call at once, +// and the box still owns every key while a correction is written: `make it 10` +// used to allow the call at its `1` and leave `make it 0` in the box. +func TestACorrectionNeverAnswersAnApprovalBesideTheCard(t *testing.T) { + a, gated, ex, now := askGated(t, nil) + a.errandEvent(ex, standingProposal(5, "remind me at 6 to leave")) + homeText(a) + *now = now.Add(time.Second) + drive(t, a, key(questionCommentKey)) + if !ex.changing { + t.Fatalf("`%s` did not arm the correction", questionCommentKey) + } + a.errandEvent(ex, ciCall("call_7")) + a.errandEvent(ex, ciConsent(7, "call_7")) + homeText(a) + *now = now.Add(time.Second) + + // TYPED THROUGH THE LOOP, so an answer a key gave would reach the agent. + for _, r := range "make it 10" { + drive(t, a, key(string(r))) + } + if len(gated.given) != 0 || len(gated.answered) != 0 { + t.Fatalf("typing the correction answered a question: approvals %+v, card %+v", gated.given, gated.answered) + } + if got := ex.box.String(); got != "make it 10" { + t.Fatalf("the correction lost characters, the box holds %q", got) + } + // AND ONCE IT IS SENT, THE APPROVAL'S DIGITS ARE ITS OWN AGAIN. + drive(t, a, key("enter")) + if len(gated.answered) != 1 || gated.answered[0].Change != "make it 10" { + t.Fatalf("enter did not send the correction, the card saw %+v", gated.answered) + } + drive(t, a, key("1")) + if len(gated.given) != 1 || !gated.given[0].allow { + t.Fatalf("after the correction `1` did not allow the call: %+v", gated.given) + } +} + +// TestTheCardSettlesAgainAfterAnApprovalHidIt is the settle guard re-armed. An +// approval hides the card's answers; when it is answered they come back under +// the hand that answered it, so a `1 1` allows the call and must not also set up +// the reminder in the same instant. +func TestTheCardSettlesAgainAfterAnApprovalHidIt(t *testing.T) { + a, gated, ex, now := askGated(t, nil) + a.errandEvent(ex, standingProposal(5, "remind me at 6 to leave")) + homeText(a) + *now = now.Add(time.Second) + a.errandEvent(ex, ciCall("call_7")) + a.errandEvent(ex, ciConsent(7, "call_7")) + homeText(a) + *now = now.Add(time.Second) + + drive(t, a, key("1")) + if len(gated.given) != 1 || !gated.given[0].allow { + t.Fatalf("`1` did not allow the call: %+v", gated.given) + } + drive(t, a, key("1")) + if len(gated.answered) != 0 { + t.Fatalf("the second `1` answered the card the instant its answers came back: %+v", gated.answered) + } + homeText(a) + *now = now.Add(questionSettle) + drive(t, a, key("1")) + if len(gated.answered) != 1 || !gated.answered[0].Approved { + t.Fatalf("a `1` on the card once it was read did not answer it: %+v", gated.answered) + } +} diff --git a/internal/tui3/homeexchange_consent_test.go b/internal/tui3/homeexchange_consent_test.go new file mode 100644 index 0000000000..ede6a3068c --- /dev/null +++ b/internal/tui3/homeexchange_consent_test.go @@ -0,0 +1,480 @@ +package tui3 + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/charmbracelet/x/ansi" + + "github.com/Agent-Field/codeaf/internal/session" +) + +// ── an approval asked inside an exchange ───────────────────────────────────── +// +// The errand's session is built with the gate allowed to ASK (cmd/codeaf's +// chatv3_local.go sets AskConsent on it, because its own standing card needs +// the same door), so a call the policy grades PROMPT emits +// session.EventConsentRequest on the errand's own stream and then BLOCKS until +// that agent is answered (internal/session's consent.go). Nothing else in this +// program holds that agent, so the pane is the only place the question can be +// answered — and these tests are what it has to do about it. + +// ciArgs is one gated call's arguments, as the engine renders them. +const ciArgs = `{"command":"gh run list --branch main"}` + +// ciRule is the policy's own sentence for asking, which the engine carries in +// Event.Rule and the question draws as its reason. +const ciRule = `bash pattern "gh *" asks first` + +// ciCall is the call the turn announces before the gate stops it. +func ciCall(callID string) session.Event { + return session.Event{ + Kind: session.EventToolBegin, Tool: "bash", CallID: callID, + Args: ciArgs, Hint: "gh run list --branch main", + } +} + +// ciConsent is the gate's request for that call, exactly as askAnswer sends +// it: the id an answer names, the call it is about, and the rule. +func ciConsent(id uint64, callID string) session.Event { + return session.Event{ + Kind: session.EventConsentRequest, ID: id, Tool: "bash", CallID: callID, + Hint: "gh run list --branch main", Args: ciArgs, Rule: ciRule, + Memo: true, Wait: session.ConsentWaiting, + } +} + +// consentGiven is one answer the errand's own agent was handed. +type consentGiven struct { + id uint64 + allow bool + scope session.ConsentScope +} + +// gatedErrand is the scripted errand whose turn STOPS ON A PERSON. It is +// [errandAgent] with the two differences that make a gate a gate: its stream +// is LEFT OPEN after the scripted events, because the engine holds the call +// until somebody answers, and it records the answers it is given — which is the +// only way a test can tell that a key in the pane reached the errand's session +// rather than the window's. +type gatedErrand struct { + *errandAgent + given []consentGiven + streams []chan session.Event +} + +func (g *gatedErrand) Submit(ctx context.Context, text string) (<-chan session.Event, error) { + g.sent = append(g.sent, text) + out := make(chan session.Event, 64) + if g.turn < len(g.turns) { + for _, event := range g.turns[g.turn] { + out <- event + } + g.turn++ + } + // THE STREAM STAYS OPEN: the turn is blocked on the call, not finished. + g.streams = append(g.streams, out) + return out, nil +} + +func (g *gatedErrand) ResolveConsent(id uint64, allow bool) { + g.ResolveConsentRemember(id, allow, session.ConsentOnce) +} + +func (g *gatedErrand) ResolveConsentRemember(id uint64, allow bool, scope session.ConsentScope) { + g.given = append(g.given, consentGiven{id: id, allow: allow, scope: scope}) +} + +// gatedApp is [errandLab.app] with the seam handing out the gated errand, its +// streams closed when the test ends so no reader is left parked on them. +func (l *errandLab) gatedApp(here string, turns ...[]session.Event) (*app, *gatedErrand) { + l.t.Helper() + a := l.app(here) + gated := &gatedErrand{errandAgent: &errandAgent{fakeAgent: fakeAgent{model: "m", turns: turns}}} + a.errand = func(orders ErrandOrders) (Agent, error) { + l.dirs = append(l.dirs, orders.Dir) + if err := os.WriteFile(filepath.Join(orders.Dir, "transcript.jsonl"), + []byte(`{"type":"session","version":1}`+"\n"), 0o600); err != nil { + return nil, err + } + return gated, nil + } + l.t.Cleanup(func() { + for _, stream := range gated.streams { + close(stream) + } + }) + return a, gated +} + +// askGated opens home on a frame wide enough to hold the pane beside the list, +// pins the clock, asks one thing whose first turn stops on the gate, and draws +// the frame once — the approval has then been on a screen, which is what the +// settle guard counts from. The clock is handed back so a test can let the +// question settle before a key. +func askGated(t *testing.T, turn []session.Event) (*app, *gatedErrand, *homeExchange, *time.Time) { + t.Helper() + lab := newErrandLab(t) + mine := lab.session("-tmp-alpha", "aaaa000000000001", "pricing research", "/tmp/alpha", time.Now()) + a, gated := lab.gatedApp(mine, turn) + besideTheList(a) + now := time.Now() + a.clock = func() time.Time { return now } + a.openHome() + typeHome(a, "what is CI doing on main") + drive(t, a, key("up"), key("enter")) + ex := theExchange(a) + if ex == nil { + t.Fatal("`ask here` opened no exchange") + } + homeText(a) + return a, gated, ex, &now +} + +// paneAnswers is what the pane drew at each target its last draw recorded for +// the question its digits answer ([homeExchange.askAt]) — the rows a press +// resolves to, read rather than searched for, so an answer named only in the +// hint under the box does not count as drawn. +func paneAnswers(a *app, ex *homeExchange) []string { + rows := a.exchangePane(ex, 60, 200, a.pal) + out := make([]string, 0, len(ex.askAt)) + for _, at := range ex.askAt { + if at < 0 || at >= len(rows) { + out = append(out, "") + continue + } + out = append(out, strings.TrimSpace(ansi.Strip(rows[at]))) + } + return out +} + +// paneRowsSaying counts the pane's own rows that say something. +func paneRowsSaying(a *app, ex *homeExchange, words string) int { + count := 0 + for _, row := range a.exchangePane(ex, 60, 200, a.pal) { + if strings.Contains(ansi.Strip(row), words) { + count++ + } + } + return count +} + +// spentStream is a stream that has already ended, for feeding an exchange the +// rest of a turn by hand: the reader the real stream left parked cannot be +// handed an event without taking it, so the remainder arrives as the messages +// the pump would have made of it (homeexchange_test.go does the same). +func spentStream() <-chan session.Event { + spent := make(chan session.Event) + close(spent) + return spent +} + +// TestAnApprovalInsideAnExchangeIsAskedAndAnsweredInThePane is the defect and +// its repair in one walk: a call made inside an `ask here` errand needs the +// person's ok, and the only place it can be given is the pane the errand is +// drawn in. +// +// IT USED TO HAVE NOWHERE TO BE ANSWERED. errandEvent folded text, tools, the +// standing card and the turn's end, and let EventConsentRequest fall through — +// so the session sat blocked on a question no screen drew, the row said +// `working` with its clock counting up for as long as the window was open, and +// a `1` pressed at it was typed into the follow-up box. +func TestAnApprovalInsideAnExchangeIsAskedAndAnsweredInThePane(t *testing.T) { + a, gated, ex, now := askGated(t, []session.Event{ + text(session.EventTextDelta, "Checking the latest run."), + ciCall("call_7"), + ciConsent(7, "call_7"), + }) + + // THE ROW SAYS IT WANTS YOU, and sorts as the thing that does. + if tail := ansi.Strip(a.exchangeTail(ex, false)); !strings.Contains(tail, homeAskWaitingWord) { + t.Fatalf("an exchange stopped on an approval says %q on its row, want %q", tail, homeAskWaitingWord) + } + if !ex.waiting() || exchangeRank(ex) != 0 { + t.Fatalf("an exchange stopped on an approval is not waiting (waiting=%v rank=%d)", ex.waiting(), exchangeRank(ex)) + } + // THE PANE ASKS IT: the gate's own sentence, the call it is about, the + // policy's reason, and the two answers — and never `always`, which would be + // a standing permission written from a forty-cell errand pane. + frame := homeText(a) + for _, want := range []string{session.ConsentHead("bash", ciArgs), "gh run list --branch main", ciRule} { + if !strings.Contains(frame, want) { + t.Fatalf("the pane does not say %q:\n%s", want, frame) + } + } + answers := paneAnswers(a, ex) + if len(answers) != 2 || !strings.Contains(answers[0], "1") || !strings.Contains(answers[0], "allow once") || + !strings.Contains(answers[1], "3") || !strings.Contains(answers[1], "deny") { + t.Fatalf("the pane's answers are %q, want `1 allow once` and `3 deny`:\n%s", answers, frame) + } + if strings.Contains(frame, "always") { + t.Fatalf("the pane offers a widening answer:\n%s", frame) + } + // A CALL WAITING ON A PERSON IS NOT A CALL DOING WORK, so the live block + // says who it is waiting on rather than `running`. + if strings.Contains(frame, homeAskRunWord) { + t.Fatalf("the pane claims the call is running while it waits for an answer:\n%s", frame) + } + // AND THE HINT NAMES EXACTLY THOSE ANSWERS. + hint := exchangeHint(ex) + if !strings.Contains(hint, "1 allow once") || !strings.Contains(hint, "3 deny") { + t.Fatalf("the hint does not name the approval's answers: %q", hint) + } + if strings.Contains(hint, "always") || strings.HasPrefix(hint, questionCommentKey+" ") || + strings.Contains(hint, " · "+questionCommentKey+" ") { + t.Fatalf("the hint names an answer the approval does not offer: %q", hint) + } + + // `1` IS ALLOW ONCE, and it reaches the ERRAND's own session. + *now = now.Add(time.Second) + drive(t, a, key("1")) + want := []consentGiven{{id: 7, allow: true, scope: session.ConsentOnce}} + if len(gated.given) != 1 || gated.given[0] != want[0] { + t.Fatalf("the errand's session was answered %+v, want %+v", gated.given, want) + } + if got := ex.box.String(); got != "" { + t.Fatalf("the answer was typed into the follow-up box as %q", got) + } + if ex.waiting() { + t.Fatal("the exchange is still waiting after its approval was answered") + } + if !ex.working { + t.Fatal("an allowed call's turn is still running, and the exchange stopped saying so") + } + if paneRowsSaying(a, ex, "allow once") != 0 || len(ex.askAt) != 0 { + t.Fatalf("the answered approval is still on the pane:\n%s", homeText(a)) + } + // THE WINDOW KEEPS NO RECEIPT OF IT. A receipt above the box belongs to + // the conversation the window is holding, and `c change` on one is answered + // by THAT engine — so an errand's decision filed there would be a key that + // re-asks it of a session that never asked it. + if len(a.questionRecords) != 0 { + t.Fatalf("the window filed a receipt for the errand's approval: %+v", a.questionRecords) + } + + // AND THE ROW KEEPS WHAT WAS DECIDED once the call comes back. + spent := spentStream() + drive(t, a, + errandEventMsg{ex: ex, ch: spent, ev: session.Event{Kind: session.EventToolEnd, Tool: "bash", CallID: "call_7", Args: ciArgs}}, + errandEventMsg{ex: ex, ch: spent, ev: text(session.EventTextDelta, "main is green.")}, + errandEventMsg{ex: ex, ch: spent, ev: session.Event{Kind: session.EventTurnDone}}, + ) + if ex.working || !ex.over() { + t.Fatalf("the turn ended and the exchange is not over (working=%v)", ex.working) + } + settled := homeText(a) + if !strings.Contains(settled, "gh run list --branch main · "+decisionWord(true)) { + t.Fatalf("the call's row does not keep the answer it was given:\n%s", settled) + } + if !strings.Contains(settled, "main is green.") { + t.Fatalf("the reply after the approval did not reach the pane:\n%s", settled) + } +} + +// TestThreeDeniesAnApprovalInThePane is the other answer: `3` refuses the call +// on the errand's session, and the row says so at once rather than spinning +// until the batch around it finishes. +func TestThreeDeniesAnApprovalInThePane(t *testing.T) { + a, gated, ex, now := askGated(t, []session.Event{ciCall("call_7"), ciConsent(7, "call_7")}) + + *now = now.Add(time.Second) + drive(t, a, key("3")) + if len(gated.given) != 1 || gated.given[0] != (consentGiven{id: 7, allow: false, scope: session.ConsentOnce}) { + t.Fatalf("the errand's session was answered %+v, want a refusal of 7", gated.given) + } + if ex.waiting() { + t.Fatal("the exchange is still waiting after its approval was refused") + } + if frame := homeText(a); !strings.Contains(frame, "gh run list --branch main · "+decisionWord(false)) { + t.Fatalf("the refused call's row does not say it was refused:\n%s", frame) + } + // AND THE ENGINE'S OWN ACCOUNT OF THE REFUSAL, which arrives when the rest + // of the step does, is the MODEL's sentence: it lands on the same row and + // leaves it saying what the person said, rather than opening a second row or + // saying `denied` twice. + drive(t, a, errandEventMsg{ex: ex, ch: spentStream(), ev: session.Event{ + Kind: session.EventToolFailed, Tool: "bash", CallID: "call_7", Args: ciArgs, + Hint: "denied by the person: " + ciRule, + }}) + tools := 0 + for _, row := range ex.rows { + if row.kind == exchangeTool { + tools++ + } + } + if tools != 1 { + t.Fatalf("the refusal opened a second row for one call: %d tool rows", tools) + } + frame := homeText(a) + if !strings.Contains(frame, "gh run list --branch main · "+decisionWord(false)) { + t.Fatalf("the refused call's row lost the person's answer:\n%s", frame) + } + if strings.Contains(frame, "denied by the person") { + t.Fatalf("the row repeats the refusal in the engine's words:\n%s", frame) + } +} + +// TestADigitThatLandsWithTheApprovalIsDropped is the block's settle guard, kept +// in the pane: a key that arrives before the question has been up for +// [questionSettle] was aimed at whatever was there before it. Here that is the +// follow-up box, so a `1` typed into a sentence must not allow a call the +// person never read. +func TestADigitThatLandsWithTheApprovalIsDropped(t *testing.T) { + a, gated, ex, now := askGated(t, []session.Event{ciCall("call_7"), ciConsent(7, "call_7")}) + + drive(t, a, key("1")) + if len(gated.given) != 0 { + t.Fatalf("a key that landed with the question answered it: %+v", gated.given) + } + if !ex.waiting() { + t.Fatal("the dropped key took the question down") + } + if got := ex.box.String(); got != "" { + t.Fatalf("the dropped key was typed into the box as %q", got) + } + *now = now.Add(questionSettle) + drive(t, a, key("1")) + if len(gated.given) != 1 || !gated.given[0].allow { + t.Fatalf("a settled `1` did not allow the call: %+v", gated.given) + } +} + +// TestTwoApprovalsInOneStepAreAskedOneAtATime is a batch that stops on two +// calls: the gate raises one question per call, both are on the pane, and the +// digits answer the oldest until it is answered. +func TestTwoApprovalsInOneStepAreAskedOneAtATime(t *testing.T) { + second := `{"command":"gh pr list"}` + a, gated, ex, now := askGated(t, []session.Event{ + ciCall("call_7"), + {Kind: session.EventToolBegin, Tool: "bash", CallID: "call_8", Args: second, Hint: "gh pr list"}, + ciConsent(7, "call_7"), + { + Kind: session.EventConsentRequest, ID: 8, Tool: "bash", CallID: "call_8", + Hint: "gh pr list", Args: second, Rule: ciRule, Memo: true, Wait: session.ConsentWaiting, + }, + }) + + if len(ex.permits) != 2 { + t.Fatalf("the pane holds %d approvals, want 2", len(ex.permits)) + } + // ONE SET OF ANSWERS ON THE SCREEN, under the call the digits answer, and + // the gate's sentence under both calls. + if drawn := paneRowsSaying(a, ex, "allow once"); drawn != 1 { + t.Fatalf("the pane draws the answers %d times, want once:\n%s", drawn, homeText(a)) + } + if heads := paneRowsSaying(a, ex, session.ConsentHead("bash", second)); heads != 2 { + t.Fatalf("the pane says the gate's sentence %d times, want once under each call:\n%s", heads, homeText(a)) + } + *now = now.Add(time.Second) + drive(t, a, key("1")) + // A SECOND KEY ON THE HEELS OF THE FIRST IS AIMED AT THE QUESTION IT JUST + // ANSWERED, not at the one that moved under it — so it is dropped until the + // next approval has been on the screen for the settle guard's quarter second. + drive(t, a, key("3")) + if len(gated.given) != 1 { + t.Fatalf("a key that landed with the second approval answered it: %+v", gated.given) + } + homeText(a) + *now = now.Add(questionSettle) + drive(t, a, key("3")) + want := []consentGiven{ + {id: 7, allow: true, scope: session.ConsentOnce}, + {id: 8, allow: false, scope: session.ConsentOnce}, + } + if len(gated.given) != 2 || gated.given[0] != want[0] || gated.given[1] != want[1] { + t.Fatalf("the two approvals were answered %+v, want %+v", gated.given, want) + } + if ex.waiting() { + t.Fatal("the exchange is still waiting after both approvals were answered") + } +} + +// TestAnApprovalLeavesThePaneWhenItsTurnEnds is the question outliving its +// call. A turn that ends while the gate is asking — interrupted, or the window +// closing — has already released the call with "not approved", so the question +// is about work that is over: it comes off the pane, the digits are characters +// again, and the exchange can settle and be filed. +func TestAnApprovalLeavesThePaneWhenItsTurnEnds(t *testing.T) { + a, gated, ex, now := askGated(t, []session.Event{ciCall("call_7"), ciConsent(7, "call_7")}) + + drive(t, a, errandClosedMsg{ex: ex}) + if ex.waiting() || len(ex.permits) != 0 { + t.Fatalf("the approval outlived its turn (waiting=%v, %d held)", ex.waiting(), len(ex.permits)) + } + if !ex.over() { + t.Fatal("an exchange whose turn ended is not over") + } + if paneRowsSaying(a, ex, "allow once") != 0 { + t.Fatalf("the approval is still drawn after its turn ended:\n%s", homeText(a)) + } + *now = now.Add(time.Second) + drive(t, a, key("1")) + if len(gated.given) != 0 { + t.Fatalf("a question nobody is waiting for was answered: %+v", gated.given) + } + if got := ex.box.String(); got != "1" { + t.Fatalf("with no question up `1` is a character, the box holds %q", got) + } +} + +// TestAClickOnAnApprovalsAnswerTakesIt is the pointer's half: each answer owns +// its whole row in the pane, and a press on it answers exactly as the digit +// does. +func TestAClickOnAnApprovalsAnswerTakesIt(t *testing.T) { + a, gated, ex, now := askGated(t, []session.Event{ciCall("call_7"), ciConsent(7, "call_7")}) + + // The draw is what writes the answers' rows, so the pane is drawn first. + homeText(a) + if len(ex.askAt) != 2 || ex.askAt[1] < 0 { + t.Fatalf("the draw did not record where the approval's answers are: %v", ex.askAt) + } + *now = now.Add(time.Second) + spend(t, a, a.exchangePress(0, ex.askAt[1])) + if len(gated.given) != 1 || gated.given[0] != (consentGiven{id: 7, allow: false, scope: session.ConsentOnce}) { + t.Fatalf("a press on `deny` answered %+v", gated.given) + } +} + +// TestAClickOnTheCardsAnswerInThePaneTakesIt holds the card to the drawing it +// now shares with the approval ([app.exchangeAnswers]): the targets a press +// resolves to are the card's own answers, and the pane leaves none of its rows +// in the block's press map, where a click in the conversation would find them. +func TestAClickOnTheCardsAnswerInThePaneTakesIt(t *testing.T) { + lab := newErrandLab(t) + mine := lab.session("-tmp-alpha", "aaaa000000000001", "pricing research", "/tmp/alpha", time.Now()) + a := lab.app(mine, []session.Event{ + standingProposal(7, "remind me at 6 to leave"), + {Kind: session.EventTurnDone}, + }) + besideTheList(a) + a.openHome() + typeHome(a, "remind me at 6 to leave") + drive(t, a, key("up"), key("enter")) + + ex := theExchange(a) + if ex == nil || !ex.asking() { + t.Fatal("the card is not asking") + } + answers := paneAnswers(a, ex) + if len(a.questionBands) != 0 { + t.Fatalf("the pane left %d rows in the block's press map", len(a.questionBands)) + } + no := -1 + for i, option := range ex.ask.question.Options { + if strings.TrimSpace(option.Key) == session.StandingNoKey { + no = i + } + } + if no < 0 || no >= len(answers) || !strings.Contains(answers[no], session.StandingNoKey) { + t.Fatalf("the card's no is not drawn at its target: answers %q", answers) + } + readCard(t, a) + spend(t, a, a.exchangePress(0, ex.askAt[no])) + if len(lab.agent.answered) != 1 || lab.agent.answered[0].Approved || lab.agent.answered[0].Once { + t.Fatalf("a press on the card's no answered %+v", lab.agent.answered) + } +} diff --git a/internal/tui3/homeexchange_test.go b/internal/tui3/homeexchange_test.go index afe66f9dcb..fc1e2d0d9c 100644 --- a/internal/tui3/homeexchange_test.go +++ b/internal/tui3/homeexchange_test.go @@ -340,6 +340,19 @@ func TestTheChordAsksHereWithoutWalkingToTheRow(t *testing.T) { } } +// readCard is a person reading the pane's card before answering it: it has been +// on the screen for the settle guard's quarter second, so a key or a press is +// taken as aimed at it ([app.questionSettled]). The tests whose subject is that +// guard drive the clock instead (homeexchange_cardguard_test.go). +func readCard(t *testing.T, a *app) { + t.Helper() + ex := theExchange(a) + if ex == nil || ex.ask == nil { + t.Fatal("there is no card up to read") + } + ex.ask.shown = a.now().Add(-questionSettle) +} + // standingProposal is one EventStandingProposal as the engine sends it. func standingProposal(id uint64, words string) session.Event { return session.Event{ @@ -385,6 +398,7 @@ func TestTheCardInThePaneIsAnsweredWithOne(t *testing.T) { t.Fatalf("the card does not say %q:\n%s", want, frame) } } + readCard(t, a) drive(t, a, key("1")) if len(lab.agent.answered) != 1 { t.Fatalf("`1` should have answered the card, the agent saw %v", lab.agent.answered) @@ -538,6 +552,7 @@ func TestTheCardInThePaneIsDeclinedWithZero(t *testing.T) { if frame := homeText(a); !strings.Contains(frame, "0 Don't remind me") { t.Fatalf("the pane does not name the decline:\n%s", frame) } + readCard(t, a) drive(t, a, key(session.StandingNoKey)) if len(lab.agent.answered) != 1 { t.Fatalf("`%s` should have answered the card, the agent saw %v", session.StandingNoKey, lab.agent.answered) @@ -954,6 +969,7 @@ func TestSayingYesHandsTheKeyboardBackToTheList(t *testing.T) { // behind the keys already in hand, so a `1` sent in the same call would be // pressed before the card it answers had arrived. drive(t, a, key("up"), key("enter")) + readCard(t, a) drive(t, a, key("1")) ex := theExchange(a) @@ -1078,6 +1094,7 @@ func TestAChangedCardIsReplacedByTheOneThatFollowsIt(t *testing.T) { a.openHome() typeHome(a, "remind me at 6 to leave") drive(t, a, key("up"), key("enter")) + readCard(t, a) drive(t, a, key(questionCommentKey)) ex := theExchange(a) @@ -1331,7 +1348,9 @@ func TestOpeningAnotherConversationLeavesTheExchangeRunning(t *testing.T) { } // AND IT IS STILL ANSWERABLE. a.home.cursor = exchangeRowAt(a, ex) - drive(t, a, key("right"), key("1")) + drive(t, a, key("right")) + readCard(t, a) + drive(t, a, key("1")) if len(lab.agent.answered) != 1 || !lab.agent.answered[0].Approved { t.Fatalf("the card outlived home but could not be answered, the agent saw %v", lab.agent.answered) } @@ -1653,6 +1672,7 @@ func TestANarrowWindowStacksTheExchangeOverTheList(t *testing.T) { } // THE CARD IS ANSWERABLE THERE. a.errandEvent(ex, standingProposal(7, "remind me at 6 to leave")) + readCard(t, a) drive(t, a, key("1")) if len(lab.agent.answered) != 1 || !lab.agent.answered[0].Approved { t.Fatalf("the card could not be answered on a narrow frame, the agent saw %v", lab.agent.answered) diff --git a/internal/tui3/question.go b/internal/tui3/question.go index 4fe57be8de..23d5728458 100644 --- a/internal/tui3/question.go +++ b/internal/tui3/question.go @@ -306,6 +306,12 @@ type questionShown struct { // another conversation — has to be told that the next enter is an answer // rather than a message, and this is where it is told. commented func() + // elsewhere says the question is about ANOTHER SESSION than the one this + // window is holding — home's errand card, answered on the errand's own agent + // (homeexchange.go). Its answer leaves no receipt above this window's box: + // that line belongs to the conversation behind it, which never asked, and the + // card already keeps its answer where it was drawn. + elsewhere bool // held is the lane's own hand on the first evidence that somebody is AT THE // KEYBOARD, and it is called once per question. // @@ -2702,8 +2708,14 @@ func (a *app) closeQuestion(q questionShown, answer session.Answer) { // that has already been made. A page is a way of READING one question; when // there is no question there is no page. a.closeQuestionPage(token) - a.recordQuestion(q, answer) - a.countQuestionYes(q, answer) + // A QUESTION ABOUT ANOTHER SESSION LEAVES NO RECEIPT HERE AND COUNTS NO YES. + // The receipt is drawn above this window's box, in a conversation that never + // asked, and the rule a third yes offers would be written for this window's + // asks out of answers given about somebody else's ([questionShown.elsewhere]). + if !q.elsewhere { + a.recordQuestion(q, answer) + a.countQuestionYes(q, answer) + } a.touch() } diff --git a/internal/tui3/tui3_test.go b/internal/tui3/tui3_test.go index b551df224e..28835a1cd4 100644 --- a/internal/tui3/tui3_test.go +++ b/internal/tui3/tui3_test.go @@ -532,6 +532,7 @@ const harnessTickSymbol = "github.com/Agent-Field/codeaf/internal/tui3.harnessTi // paying the old price. var blockingCommands = []string{ "waitDesign", + "waitErrand", "waitEvent", "waitGuestNotices", "waitGuestQuestions",