✨ harness: ask_user tool — the agent can stop and ask the human (ACP elicitation)

[ibolton336]

Why

A question the agent writes in prose is never seen until the run is over, and a turn that ends on a question is a finished run. The tee (#96) lets a human watch and steer, but steer is a nudge into a running turn — it cannot make the agent wait. Seen live today: an execute-stage run found no PLAN.md, asked "Would you like me to 1/2/3?" in prose, then answered itself and wandered off to do the plan stage; a "pause for a status report" steer was read and then it carried on.

This gives the agent a real in-turn stop and confirm — the "how does interactive input reach the agent" piece of #55 — using ACP's own primitive for it.

What

• ask_user tool. internal/askuser is a minimal stdio MCP server that is the harness binary itself (migration-harness ask-user-mcp, hidden subcommand), exposing one tool: ask_user(question, options?). A call becomes an MCP elicitation/create back to goose with a flat form schema (required string answer, enum when options are given). The tool result tells the model "The human answered: …", or that the human declined / nobody answered — never an invented answer. No new image dependencies: the harness lists itself in session/new mcpServers.
• Harness ACP client advertises clientCapabilities.elicitation.form (goose ≥ 1.45 gates relaying MCP elicitation on it), and elicitation/create asks go through the same forwarder as permission asks (ForwardElicitation). Fail closed everywhere: no viewer, no answer within HARNESS_HITL_TIMEOUT_SECONDS, or no tee → {action: "cancel"}. MCPServer takes the ACP stdio shape (name + env as a name/value list).
• Tee relays questions to viewers under kask-<n> ids (permission asks keep kperm-<n>; first answer wins, answers intercepted off the viewer pipe), and replays a pending ask to a viewer that attaches mid-question — a question waits minutes, and the harness status ring alone would not carry it.
• Prompt guideline (only when the tool is mounted): call ask_user for decisions only a human can make, don't ask in prose; and if a human's mid-turn redirect asks to pause / check in / report status / wait, call ask_user with status + choices and do not continue until it answers — so steer and the blocking question compose.
• HARNESS_HITL_ASK=off leaves the tool out (default on, like HARNESS_HITL_STEER). README + changelog fragment included.

sequenceDiagram
  participant M as model
  participant G as goose
  participant S as ask_user MCP (harness binary)
  participant H as harness ACP client
  participant T as tee
  participant V as viewer (console)
  M->>G: tools/call ask_user(question, options)
  G->>S: tools/call
  S->>G: elicitation/create (message + form schema)
  G->>H: elicitation/create (ACP, client advertises elicitation.form)
  H->>T: ForwardElicitation
  T->>V: elicitation/create id kask-1 (replayed to late viewers)
  V->>T: result action accept, content answer
  T->>H: answer
  H->>G: result (verbatim)
  G->>S: elicitation result
  S->>G: tool result: "The human answered: …"
  G->>M: tool result (turn was blocked the whole time)

Loading

Verification

• Unit: askuser round trip over pipes (initialize → tools/list → tools/call → elicitation → accept / decline / cancel / error / EOF), session elicitation relay + fail-closed matrix, tee relay with late-viewer replay, prompt guideline placement. Whole harness green with -race.
• Live (integration tag, real goose 1.45 + Bedrock, TestAskUserBlocksLiveRun): the model called ask_user, the question reached the viewer as kask-1 with the enum schema, the turn stayed blocked until the viewer answered postgres, and the final sentence named it.
• On a cluster (ROKS, agent image built from this branch, console rendering the ask as a form card): the question card appears, the agent waits, answering resumes it; a "pause for a status report" steer now turns into a status question with choices.

Not in this PR / follow-ups

• Other viewers' cards do not learn when someone else answered (the tee could broadcast a resolution frame).
• Questions share the permission timeout (HARNESS_HITL_TIMEOUT_SECONDS, 180 s default); a question probably wants its own, longer default.
• Read-only vs write authorization on the stream is the Hub design question (tackle2-hub#1116); answering a question is solicited input, like answering a permission ask, and is treated the same way here.

HITL.mov

Refs #55, #56, #96.

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: c421cab2-4a14-4d64-8953-8d91b564c8e8

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[savitharaghunathan]

Thanks for the PR, Ian. This is great.

One quick question: do we need to record these design decisions in an ADR somewhere? Might make it easier when we add new agent runtime support (if and when) based on usage.

Left a few inline notes below on edge cases surfaced during review — none blocking, but worth a look.

[savitharaghunathan]

Race: replayed asks can outlive their answer

resolveAsk() deletes s.perms[id] and sends the result on ch before forwardAsk()'s deferred cleanup (delete(s.perms, id); delete(s.pendingAsks, id)) runs. A viewer that attaches in that gap gets the already-answered question replayed from pendingAsks via serveViewer's backlog snapshot. If they answer it, resolveAsk() finds no entry in s.perms and silently drops the reply ("late/duplicate answer... ignoring") — the viewer gets no feedback that their answer went nowhere.

Suggest deleting from pendingAsks synchronously at the point the answer is sent/resolved (before the channel send), rather than in the deferred cleanup after forwardAsk wakes up, so there's no window where an already-resolved ask is still visible to new viewers.

[ibolton336]

Good catch — the window between resolveAsk's channel send and forwardAsk's deferred cleanup is real. Fixed in 5d21888 exactly as you suggest: the pendingAsks frame is dropped in the same critical section that commits the answer (the deferred cleanup still covers timeout/shutdown), with a test pinning the invariant.

[savitharaghunathan]

Byte-offset truncation can split a UTF-8 rune

title := params.Message
if len(title) > 80 {
    title = title[:77] + "..."
}

len() and slicing here operate on bytes, not runes. A non-ASCII message (accented chars, CJK, emoji) cut at byte 77 can land mid-rune, producing invalid UTF-8 in the log line.

Suggest truncating on a rune boundary, e.g. []rune(title) with a rune-count check, or utf8.RuneCountInString + iterating rune boundaries — or pull in a small helper like strings.ToValidUTF8 after truncation as a cheap safety net.

[ibolton336]

Fixed in 5d21888 — truncation now happens on rune boundaries ([]rune slice), so a non-ASCII question can't be cut mid-sequence.

[savitharaghunathan]

omitempty was dropped in the map→slice migration; Env can now marshal as null

The comment above this struct explains the switch from map[string]string to []EnvVar exists specifically because goose's untagged-enum MCP-server parser rejects a bare map (-32602). But Env lost its omitempty tag, and nothing guarantees a caller sets it — a nil slice marshals to "env":null, which risks tripping the exact same parser failure this refactor was meant to fix.

Today's two call sites (main.go, integration test) happen to pass Env: []acp.EnvVar{} explicitly, so this is latent rather than active. Suggest either restoring omitempty (if null/missing is actually accepted) or adding a constructor/MarshalJSON that defaults nil to []EnvVar{}, so future call sites can't silently regress this.

[ibolton336]

Fixed in 5d21888, via your second option: MCPServer.MarshalJSON normalizes nil to []. I deliberately avoided omitempty — the untagged parse is only proven against the present-and-array shape, and an absent field is as unproven against it as null. Args had the same latent shape, so it's normalized too, and a test locks the wire shape.

[ibolton336]

Thanks @savitharaghunathan — good call on the ADR. Added as docs/adr/0017-ask-user-tool-and-elicitation.md (e966445, renumbered 0016→0017 in 8b9126c — #145 had already claimed 0016): it records the decisions (asking is a tool call, elicitation as the carrier, the kask-*/kperm-* split, fail-closed policy) and — for exactly the reason you gave — a three-point acceptance checklist a future agent runtime must meet to compose with this flow.

All three inline notes are addressed in 5d21888; replies on each thread. Harness suite green with -race.

[fabianvf]

elicitation landed in 2025-06-18 (https://modelcontextprotocol.io/specification/2025-06-18/changelog, item 6), but this echoes back any version that sorts <= ours, so a client initializing at 2025-03-26 gets agreement and then an elicitation/create that doesn't exist in that revision. The Containerfile pulls goose from releases/download/stable, so which version we get isn't pinned either. Since the whole server is elicitation, should it just always answer protocolVersion?

[ibolton336]

Yes — agreed. Fixed in 194b8aa: initialize now always answers 2025-06-18, whatever the client offers. Agreeing to an older revision promised a protocol this server can't keep — elicitation is the whole server — so a pre-elicitation client now fails at the handshake (with the mismatch logged) instead of on the first elicitation/create mid-turn. A test pins the 2025-03-26 offer.

Agreed on the unpinned goose pull too — that predates this PR (agent-base pulls stable), so I'd rather split it into a follow-up issue than grow this one. Happy to file it.