diff --git a/CHANGELOG.md b/CHANGELOG.md index 278995c..8ce0f82 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -161,6 +161,15 @@ for every approved plan. Desktop's version follows the engine generation, so it states plainly that it wipes context completely, so the two are not mistaken for each other. ### Changed +- **Asking another agent no longer locks up the one that asked.** Asking used to wait for the other + agent's whole reply, so a question that turned out to be several minutes of work held your agent + the entire time and you could not type into it. Asking now hands off and returns immediately, the + same way handing over a job already did: your agent stays available, the reply appears in the + conversation as soon as it arrives, and your agent has it on its next turn. Replies are worded as + replies rather than as finished jobs, and a reply gets far more room on the card than a job's + one-line outcome — for a question the answer is the thing you wanted, so it is shown rather than + summarised. An agent that is busy still says so up front and points you at reading its + conversation instead. - **The bundled backgrounds are renumbered to put the new default first.** Golden Gate, Sequoia Trail, and Pismo Beach are unchanged and still in the gallery, but each has moved down one position. If you had picked one of them, your background keeps working exactly as before — the diff --git a/docs/agent-mentions.md b/docs/agent-mentions.md index 0f5f4be..a238923 100644 --- a/docs/agent-mentions.md +++ b/docs/agent-mentions.md @@ -28,11 +28,14 @@ The second insight is that **most of what you want does not require waking the o | `agent_status` | no | **yes** | "are you done", "what are you on" | | `review_agent_work` | no | **yes** | "is the work any good" | | `read_agent_transcript` | no | **yes** | "what exactly was said" | -| `ask_agent` | **yes** | no | judgment, explanation, intent | +| `ask_agent` | **yes** (in the background) | no | judgment, explanation, intent | Three of four are pure observation. They carry no loop risk, no concurrency problem and no approval question, because nothing runs. All the hazard is concentrated in the fourth. +`ask_agent` wakes the target but does not block the caller — it hands off and returns at once, the +same as `delegate_to_agent`. See "Delegation, the inbox, and the rolling digest" below. + ## Build order Each step is useful alone and complexity rises monotonically. Stopping after any of them leaves a @@ -109,7 +112,8 @@ Open problems, all of which belong to this tool alone: mid-turn, `ask_agent` returns its status plus a pointer to `read_agent_transcript`, so the busy case degrades to reading rather than dead-ending. This matters because busy is the COMMON case: "have you finished X?" is asked precisely when the answer might be no. Queueing was rejected — a - caller that waited would stall its own turn behind work of unknown length. + caller that waited would stall its own turn behind work of unknown length. (Since 2026-09-10 this + check runs BEFORE the hand-off rather than on the awaited result; see the delegation section.) - **Acting vs answering — settled: a delegated turn is a FULL turn.** The target answers with all of its own tools, exactly as it would answer the user, because the point of asking a colleague is that they can go and look. A read-only delegated turn would make the feature useless for what it @@ -192,12 +196,37 @@ target's transcript; `ask_agent` produces a real turn there and must be attribut ## Delegation, the inbox, and the rolling digest -`ask_agent` blocks. For a question that is right; for a job — "build a website" — it holds the -asking agent's turn gate for minutes, and messages to that agent are dropped while it waits. Two -separate problems: the asker is locked, and agents cannot wake up to report anything. - -**`delegate_to_agent`** returns immediately. The job runs in the background on the target, and the -asking agent's turn ends at once, so the user keeps their agent. +**Superseded 2026-09-10 — `ask_agent` no longer blocks.** The original reasoning below is kept +because it is why `delegate_to_agent` exists, and the distinction it draws turned out to be wrong +in a way worth recording. + +> `ask_agent` blocks. For a question that is right; for a job — "build a website" — it holds the +> asking agent's turn gate for minutes, and messages to that agent are dropped while it waits. Two +> separate problems: the asker is locked, and agents cannot wake up to report anything. + +The error was treating "question" and "job" as different in kind. They are not: both run one full +turn on the target, through the same `AskAsync`. The only difference was whether the caller awaited +it — and that made the model responsible for predicting, before asking, whether a question would +turn out to be quick. That is the judgement it is worst at, and the cost of getting it wrong was +the user locked out of their own agent with no way to convert or cancel. + +**Both now hand off and return at once.** `ask_agent` opens a `Delegation` exactly as +`delegate_to_agent` does, marked `DelegationKind.Question`, and the kind changes only the WORDING of +the result — "Ninja replied" rather than "Ninja finished", and a larger slice of the reply on the +card, because for a question the reply is the deliverable rather than a note about work that lives +in the files. The asking agent's turn ends immediately in both cases. + +Two consequences worth stating plainly: + +- **The busy check moved earlier.** While asking blocked, the target's atomic claim decided and the + tool only worded the refusal. With nobody waiting to hear that, a busy target must be caught + before the hand-off — otherwise the model is told its question is on its way and learns otherwise + from a failure card much later. The claim inside `AskAsync` is still the authority; a race between + the two now completes the question as unanswered rather than returning a refusal. +- **The loop guard now crosses a thread boundary.** `AgentCallChain` is an `AsyncLocal`, and the + chain reaches the target's turn only because `Task.Run` captures `ExecutionContext`. If that ever + stopped holding, the guard would fail silently — so it is pinned by a test that asserts the chain + is visible on the far side of the hand-off, rather than left to inspection. **Each agent has an inbox** (`AgentInbox`). Anything that happens while an agent is idle waits there and is folded into the preamble of its next turn — the same ride-along `_armedContexts` diff --git a/src/MandoCode.Desktop.Tests/AgentAskTests.cs b/src/MandoCode.Desktop.Tests/AgentAskTests.cs index 739ce62..8ee6768 100644 --- a/src/MandoCode.Desktop.Tests/AgentAskTests.cs +++ b/src/MandoCode.Desktop.Tests/AgentAskTests.cs @@ -4,9 +4,14 @@ namespace MandoCode.Desktop.Tests; /// -/// Asking another agent a real question. This is the only cross-agent tool that runs a turn on -/// someone else's model, so it is the only one that can loop, collide with a busy agent, or spend -/// tokens the user did not ask for — everything here is about those three. +/// Asking another agent a real question, and handing one a job. These are the two cross-agent tools +/// that run a turn on someone else's model, so they are the only ones that can loop, collide with a +/// busy agent, or spend tokens the user did not ask for — most of what follows is about those three. +/// +/// Neither BLOCKS. Both hand off and return at once, so the asking agent stays available to +/// the user; the reply arrives through the inbox. The tests that pin "returns immediately" are the +/// point of the feature, not incidental detail — asking used to wait for the answer, which locked +/// the user out of their own agent for as long as the other one took. /// public class AgentAskTests { @@ -37,14 +42,25 @@ public Task AskAsync(string askedBy, string question, CancellationTo private static AgentEntry Entry(string key, string name, bool busy = false, int step = 0, int total = 0) => new(key, name, $@"C:\src\{name}", name.ToLowerInvariant(), "qwen3:8b", busy, false, step, total); - private static (AgentDirectoryTools Tools, FakePeer Peer, AgentDirectory Dir) Setup( + /// The hand-off is captured rather than run, so a test sees exactly what WOULD be + /// started without a background turn racing its assertions. Started.Count being 0 is how + /// "nothing was set going" is proved. + private sealed class Launches + { + public List All { get; } = new(); + public Delegation? Last => All.Count == 0 ? null : All[^1]; + } + + private static (AgentDirectoryTools Tools, FakePeer Peer, AgentDirectory Dir, Launches Started) Setup( bool busy = false, string answer = "done both") { var dir = new AgentDirectory(); dir.Replace(new[] { Entry("k1", "Ninja", busy, step: 4, total: 9), Entry(Self, "Sonic") }); var peer = new FakePeer("k1", answer, busy); dir.RegisterPeer(peer); - return (new AgentDirectoryTools(dir, Self, () => "Sonic"), peer, dir); + var started = new Launches(); + var tools = new AgentDirectoryTools(dir, Self, () => "Sonic", (d, _) => started.All.Add(d)); + return (tools, peer, dir, started); } [Fact] @@ -108,76 +124,108 @@ public async Task CheckingDelegationsReportsEachJobWithoutAskingAnyone() [Fact] public async Task WithNothingDelegatedTheReportSaysSoPlainly() { - var (tools, _, _) = Setup(); + var (tools, _, _, _) = Setup(); Assert.Contains("not handed any work", tools.CheckDelegations()); await Task.CompletedTask; } [Fact] - public async Task AnIdleAgentIsAskedAndItsAnswerComesBack() + public void AskingReturnsAtOnceAndDoesNotWaitForTheAnswer() { - var (tools, peer, _) = Setup(answer: "yes, both finished"); + // The whole point of the change. Asking used to await the far side's entire turn, so a + // question that turned out to be ten minutes of work pinned the asking agent for ten + // minutes and the user could not type into their own tab. + var (tools, peer, _, started) = Setup(answer: "yes, both finished"); + + var result = tools.AskAgent("Ninja", "have you finished the X and Y tasks?"); + + Assert.Equal(0, peer.Asked); // nothing awaited on this thread + Assert.Single(started.All); + Assert.Equal("have you finished the X and Y tasks?", started.Last!.Task); + Assert.Equal(DelegationKind.Question, started.Last!.Kind); + Assert.DoesNotContain("yes, both finished", result); // the answer cannot be known yet + } - var result = await tools.AskAgent("Ninja", "have you finished the X and Y tasks?"); + [Fact] + public void TheAskerIsToldNotToWaitOrGuessTheAnswer() + { + // The model is perfectly capable of "I'll wait for Ninja" — or worse, inventing what Ninja + // is going to say. The tool result is the only place it learns neither is available. + var (tools, _, _, _) = Setup(); - Assert.Equal(1, peer.Asked); - Assert.Equal("have you finished the X and Y tasks?", peer.LastQuestion); - Assert.Contains("yes, both finished", result); - Assert.Contains("Ninja replied", result); + var result = tools.AskAgent("Ninja", "which port does the dev server use?"); + + Assert.Contains("next turn", result); + Assert.Contains("rather than waiting", result); + Assert.Contains("do not guess", result); } [Fact] - public async Task TheAnswerIsAttributedToTheAgentDoingTheAsking() + public void TheQuestionCarriesTheAskingAgentsName() { // The target shows this in its own transcript. Unattributed, a question would read as - // something the user typed. - var (tools, peer, _) = Setup(); + // something the user typed. The name now travels on the delegation rather than straight + // into AskAsync, because the call that reaches the peer happens later, on another thread. + var (tools, _, _, started) = Setup(); - await tools.AskAgent("Ninja", "status?"); + tools.AskAgent("Ninja", "status?"); - Assert.Equal("Sonic", peer.LastAskedBy); + Assert.Equal("Sonic", started.Last!.FromName); + Assert.Equal("Ninja", started.Last!.ToName); } [Fact] - public async Task ABusyAgentDeclinesAndTheCallerIsPointedAtTheReadTools() + public void ABusyAgentIsNotAskedAndTheCallerIsPointedAtTheReadTools() { // The busy case is the COMMON case — "have you finished" is asked precisely when the answer // might be no. A bare refusal would end the line of enquiry; naming the fallback keeps it // going with what can be read without interrupting anyone. // - // Note the peer IS asked: the claim is what decides, not a flag read beforehand. Checking - // first left a window where the agent could take a turn in between. - var (tools, peer, _) = Setup(busy: true); + // This check MOVED. While asking blocked, the peer's atomic claim decided and this layer + // only worded the refusal. Handing off means nobody is waiting to hear it, so busy has to + // be caught before the hand-off — otherwise the model is told its question is on its way + // and finds out it was not from a failure card much later. + var (tools, peer, _, started) = Setup(busy: true); - var result = await tools.AskAgent("Ninja", "done yet?"); + var result = tools.AskAgent("Ninja", "done yet?"); - Assert.Equal(1, peer.Asked); - Assert.Contains("could not answer", result); + Assert.Equal(0, peer.Asked); + Assert.Empty(started.All); + Assert.Contains("busy", result); Assert.Contains("step 4 of 9", result); Assert.Contains("read_agent_transcript", result); } [Fact] - public async Task AnAgentThatBecomesBusyBetweenTheHintAndTheAskStillDeclinesCleanly() + public async Task AnAgentThatGoesBusyAfterTheHandOffCompletesTheQuestionUnanswered() { - // The race the atomic claim exists for: the directory says idle, and the agent takes a turn - // before the question lands. The far side must decline rather than run a second turn on the - // same chat history, and the caller must still get the useful refusal. - var (tools, peer, _) = Setup(busy: false); - peer.IsBusy = true; // as if a turn started in the gap + // The race the atomic claim still exists for: the up-front check passes, and the agent takes + // a turn before the background call lands. The far side must decline rather than run a + // second turn on the same chat history — and because nobody is waiting on this path, the + // refusal has to surface as a COMPLETED-but-unanswered delegation rather than a return value. + var dir = new AgentDirectory(); + dir.Replace(new[] { Entry("k1", "Ninja", step: 4, total: 9), Entry(Self, "Sonic") }); + var peer = new FakePeer("k1"); + dir.RegisterPeer(peer); - var result = await tools.AskAgent("Ninja", "done yet?"); + PeerAnswer? outcome = null; + var tools = new AgentDirectoryTools(dir, Self, () => "Sonic", + (d, target) => { peer.IsBusy = true; outcome = target.AskAsync(d.FromName, d.Task).Result; }); - Assert.Contains("could not answer", result); - Assert.Contains("read_agent_transcript", result); + tools.AskAgent("Ninja", "done yet?"); + + Assert.Equal(1, peer.Asked); + Assert.False(outcome!.Answered); + Assert.Equal("busy", outcome!.Text); + await Task.CompletedTask; } [Fact] public async Task AnUnknownAgentNamesTheOnesThatExist() { - var (tools, peer, _) = Setup(); + var (tools, peer, _, _) = Setup(); - var result = await tools.AskAgent("Knuckles", "hello?"); + var result = tools.AskAgent("Knuckles", "hello?"); Assert.Equal(0, peer.Asked); Assert.Contains("Ninja", result); @@ -186,8 +234,8 @@ public async Task AnUnknownAgentNamesTheOnesThatExist() [Fact] public async Task AskingYourselfIsRefused() { - var (tools, peer, _) = Setup(); - Assert.Contains("is you", await tools.AskAgent("Sonic", "what am I doing?")); + var (tools, peer, _, _) = Setup(); + Assert.Contains("is you", tools.AskAgent("Sonic", "what am I doing?")); Assert.Equal(0, peer.Asked); } @@ -196,10 +244,10 @@ public async Task AClosedTabCannotBeAsked() { // The display snapshot can still name an agent whose tab has gone. Resolving must not // produce a reference to something that no longer exists. - var (tools, peer, dir) = Setup(); + var (tools, peer, dir, _) = Setup(); dir.RemovePeer("k1"); - var result = await tools.AskAgent("Ninja", "still there?"); + var result = tools.AskAgent("Ninja", "still there?"); Assert.Equal(0, peer.Asked); Assert.Contains("closed", result); @@ -210,11 +258,11 @@ public async Task AnAgentAlreadyInTheChainIsNotAskedAgain() { // Sonic asks Ninja; while answering, Ninja asks Sonic back. Without the guard the two // continue until the budget is gone. - var (tools, peer, _) = Setup(); + var (tools, peer, _, _) = Setup(); using (AgentCallChain.Enter("k1")) { - var result = await tools.AskAgent("Ninja", "and you?"); + var result = tools.AskAgent("Ninja", "and you?"); Assert.Equal(0, peer.Asked); Assert.Contains("loop", result); } @@ -225,12 +273,12 @@ public async Task AChainStopsAtTheDepthLimitEvenWithoutRepeatingAnAgent() { // A → B → C → D never repeats anyone, so the cycle check alone would let it run as far as // there are agents. The depth cap is what bounds the cost. - var (tools, peer, _) = Setup(); + var (tools, peer, _, _) = Setup(); using (AgentCallChain.Enter("other-1")) using (AgentCallChain.Enter("other-2")) { - var result = await tools.AskAgent("Ninja", "one more?"); + var result = tools.AskAgent("Ninja", "one more?"); Assert.Equal(0, peer.Asked); Assert.Contains("limit", result); } @@ -246,7 +294,7 @@ public async Task TheGuardSurvivesTheAwaitsAndThreadHopsOfARealCall() // // So this deliberately crosses the boundaries a real call crosses: an await, a thread-pool // hop, and a continuation that does not capture context. - var (tools, peer, _) = Setup(); + var (tools, peer, _, _) = Setup(); using (AgentCallChain.Enter("k1")) { @@ -254,7 +302,7 @@ public async Task TheGuardSurvivesTheAwaitsAndThreadHopsOfARealCall() await Task.Run(async () => { await Task.Delay(1).ConfigureAwait(false); - var result = await tools.AskAgent("Ninja", "still looping?").ConfigureAwait(false); + var result = tools.AskAgent("Ninja", "still looping?"); Assert.Contains("loop", result); }).ConfigureAwait(false); } @@ -268,7 +316,7 @@ public async Task TwoChainsRunningAtOnceDoNotSpendEachOthersBudget() // Two conversations can ask questions at the same moment. A shared field would let one // chain's depth block the other's first question — the reason this is an AsyncLocal and not // a static counter. Verified by running both concurrently rather than in sequence. - var (tools, peer, _) = Setup(); + var (tools, peer, _, _) = Setup(); var blocked = Task.Run(async () => { @@ -276,21 +324,21 @@ public async Task TwoChainsRunningAtOnceDoNotSpendEachOthersBudget() using (AgentCallChain.Enter("other-2")) { await Task.Delay(5).ConfigureAwait(false); - return await tools.AskAgent("Ninja", "deep chain").ConfigureAwait(false); + return tools.AskAgent("Ninja", "deep chain"); } }); var allowed = Task.Run(async () => { await Task.Delay(5).ConfigureAwait(false); - return await tools.AskAgent("Ninja", "fresh chain").ConfigureAwait(false); + return tools.AskAgent("Ninja", "fresh chain"); }); var results = await Task.WhenAll(blocked, allowed); - Assert.Contains("limit", results[0]); // the deep chain is stopped - Assert.Contains("Ninja replied", results[1]); // the independent one is not - Assert.Equal(1, peer.Asked); + Assert.Contains("limit", results[0]); // the deep chain is stopped + Assert.Contains("Asked Ninja", results[1]); // the independent one is not + Assert.Equal(0, peer.Asked); // handed off, not awaited here } [Fact] @@ -298,14 +346,130 @@ public async Task TheChainUnwindsSoLaterQuestionsAreNotBlocked() { // The guard is scoped to one chain. A question asked after an earlier chain finished must // start from a clean budget, or the first cross-agent call of a session would poison the rest. - var (tools, peer, _) = Setup(); + var (tools, peer, _, started) = Setup(); using (AgentCallChain.Enter("other-1")) using (AgentCallChain.Enter("other-2")) { } - var result = await tools.AskAgent("Ninja", "now?"); + var result = tools.AskAgent("Ninja", "now?"); - Assert.Equal(1, peer.Asked); - Assert.Contains("Ninja replied", result); + Assert.Equal(0, peer.Asked); // handed off, not awaited here + Assert.Single(started.All); + Assert.Contains("Asked Ninja", result); + } + + [Fact] + public async Task TheLoopGuardSurvivesTheHandOffIntoABackgroundTurn() + { + // THE risk this change introduces. Asking used to await the far side inside the caller's + // own async flow, so the chain was plainly still in scope. It now hands off to a + // Task.Run in AgentSession.StartDelegation, and the guard only keeps working because + // Task.Run captures ExecutionContext and an AsyncLocal rides along with it. + // + // If that ever stopped being true the guard would fail SILENTLY — no exception, no failing + // test elsewhere, just agents talking in circles until the budget is gone. So this asserts + // the chain is visible on the far side of exactly that boundary. + var dir = new AgentDirectory(); + dir.Replace(new[] { Entry("k1", "Ninja"), Entry(Self, "Sonic") }); + dir.RegisterPeer(new FakePeer("k1")); + + string? seenInsideBackgroundTurn = null; + var gate = new TaskCompletionSource(); + + // Mirrors StartDelegation: fire-and-forget onto the pool, then Enter the target's key the + // way SessionAgentPeer.AskAsync does before running the turn. + var tools = new AgentDirectoryTools(dir, Self, () => "Sonic", (delegation, unusedPeer) => + { + _ = unusedPeer; + _ = Task.Run(() => + { + using (AgentCallChain.Enter(delegation.ToKey)) + seenInsideBackgroundTurn = AgentCallChain.Reject("upstream", "Upstream"); + gate.SetResult(); + }); + }); + + // Sonic is answering Upstream when it asks Ninja — so "upstream" is already in the chain. + using (AgentCallChain.Enter("upstream")) + tools.AskAgent("Ninja", "what is the schema?"); + + await gate.Task; + + // Inside the background turn the chain must be [upstream, k1] — so asking Upstream back + // is refused as a loop. A null here means the chain was lost crossing the hand-off. + Assert.NotNull(seenInsideBackgroundTurn); + Assert.Contains("loop", seenInsideBackgroundTurn!); + } + + // ---- how a finished question is worded back ---------------------------------- + + private static Delegation Finished(DelegationKind kind, string task, string result) => + new("d1", Self, "Sonic", "k1", "Ninja", task, DateTimeOffset.Now.AddMinutes(-2), + DelegationState.Done, result, DateTimeOffset.Now, kind); + + [Fact] + public void AnAnsweredQuestionReadsAsAReplyNotAFinishedJob() + { + // Same machinery carries both, so without the kind a question asked in passing would come + // back as "Ninja finished ..." — which reads as though work was done on the user's behalf. + var line = DelegationRegistry.CompletionLine( + Finished(DelegationKind.Question, "which port does the dev server use?", "It's on 5173.")); + + Assert.Contains("replied", line); + Assert.Contains("It's on 5173.", line); + Assert.DoesNotContain("finished", line); + } + + [Fact] + public void AFinishedJobStillReadsAsFinished() + { + // The other half of the same guarantee: adding the question wording must not reword jobs. + var line = DelegationRegistry.CompletionLine( + Finished(DelegationKind.Job, "build the marketing site", "Done. Deployed to staging.")); + + Assert.Contains("finished", line); + Assert.DoesNotContain("replied", line); + } + + [Fact] + public void AnAnswerIsGivenMoreRoomOnTheCardThanAJobsOutcome() + { + // For a job the interesting thing is THAT it finished; the work is in the files. For a + // question the reply IS the deliverable, so clipping it to a job's length would send the + // user to the other agent's tab to read two sentences. + var answer = string.Join(" ", Enumerable.Repeat("the schema uses snake_case throughout", 20)); + + var asked = DelegationRegistry.CompletionLine(Finished(DelegationKind.Question, "schema?", answer)); + var job = DelegationRegistry.CompletionLine(Finished(DelegationKind.Job, "schema?", answer)); + + Assert.True(asked.Length > job.Length, $"answer card {asked.Length} should exceed job card {job.Length}"); + } + + [Fact] + public void TheInboxCarriesTheWholeAnswerEvenWhenTheCardTrimsIt() + { + // The card is for the user and is one line. The inbox copy is what the MODEL reads on its + // next turn, so trimming there would make the clipped version the only one it ever sees. + var answer = string.Join(" ", Enumerable.Repeat("a very specific detail that matters", 40)); + var d = Finished(DelegationKind.Question, "schema?", answer); + + var digest = DelegationRegistry.Digest(d, Entry("k1", "Ninja"), Array.Empty()); + + Assert.Contains(answer, digest.Body); + Assert.Contains("replied", digest.Body); + Assert.Contains("answered your question", digest.Subject); + } + + [Fact] + public void AQuestionStillWaitingReadsAsAnAnswerComingNotWorkInProgress() + { + var d = new Delegation("d1", Self, "Sonic", "k1", "Ninja", "schema?", + DateTimeOffset.Now.AddMinutes(-1), Kind: DelegationKind.Question); + + var digest = DelegationRegistry.Digest(d, Entry("k1", "Ninja", busy: true, step: 2, total: 5), + Array.Empty()); + + Assert.Contains("working out an answer", digest.Body); + Assert.Contains("step 2 of 5", digest.Body); } } diff --git a/src/MandoCode.Desktop/Services/AgentDirectoryTools.cs b/src/MandoCode.Desktop/Services/AgentDirectoryTools.cs index a386bbd..b80fb6d 100644 --- a/src/MandoCode.Desktop/Services/AgentDirectoryTools.cs +++ b/src/MandoCode.Desktop/Services/AgentDirectoryTools.cs @@ -7,10 +7,17 @@ namespace MandoCode.Desktop.Services; /// preview tools use. Host-owned rather than engine-owned because only the host knows what other /// agents exist; an agent's own tools stay bounded to its own project root. /// -/// Everything here is OBSERVATION. Nothing in this class runs a turn on another agent's model, -/// which is why none of it can loop, none of it collides with another agent being busy, and none of -/// it raises an approval question. Asking another agent a real question is a separate tool with a -/// genuinely different risk profile — see docs/agent-mentions.md. +/// Two kinds of tool live here, and the difference is the whole design. , +/// , and +/// are pure OBSERVATION: they read state the host already keeps, so they cost no turn, cannot loop, +/// work while the other agent is busy, and raise no approval question. and +/// genuinely run a turn on another agent's model, so both are guarded +/// by the loop check and both refuse a busy target. +/// +/// NEITHER of those two blocks. Both hand the work off and return at once, so this agent stays +/// free for the user while the other works; the result comes back through the inbox and is announced +/// in this agent's transcript. They differ only in how that result is worded — see +/// and docs/agent-mentions.md. /// public sealed class AgentDirectoryTools { @@ -73,13 +80,16 @@ public string GetAgentStatus( return AgentDirectory.Describe(agent); } - [Description("Asks another agent a question and returns its answer in its own words. The other " + - "agent answers from what it has been working on and can use its own tools to check, " + - "so prefer this over reading its transcript when you want a specific answer. Only " + - "works when that agent is idle: if it is busy, this returns a note saying so, and you " + - "should call get_agent_status and read_agent_transcript instead to work it out from " + - "what it has already done.")] - public async Task AskAgent( + [Description("Asks another agent a question and returns immediately, without waiting for the " + + "answer. The other agent answers from what it has been working on and can use its " + + "own tools to check, so prefer this over reading its transcript when you want a " + + "specific answer. You stay free to keep talking to the user meanwhile: the reply " + + "appears in the conversation as soon as it arrives, and you are given it on your " + + "next turn — so do not promise to wait, and do not claim to know the answer yet. " + + "Only works when that agent is idle: if it is busy, this says so, and you should " + + "call get_agent_status and read_agent_transcript instead to work it out from what " + + "it has already done.")] + public string AskAgent( [Description("The agent's name, as the user typed it after '@'.")] string name, [Description("The question, phrased as you would ask a colleague. Be specific — the other " + "agent cannot see your conversation.")] string question) @@ -92,20 +102,30 @@ public async Task AskAgent( var peer = _directory.Peer(agent.Key); if (peer == null) return $"{agent.Name}'s tab has closed — it cannot be asked anything now."; + // Busy is now checked HERE, up front, which it was not while this call blocked. Back then + // the peer's atomic claim reported the refusal through the awaited result, and this layer + // only worded it. Handing off means nobody is waiting to hear that, so a busy target has to + // be caught before the hand-off or the model would be told its question was on its way and + // learn otherwise only from a failure card. The claim inside AskAsync is still the + // authority — this is the early, well-worded half, and a race between the two simply + // completes the question as unanswered. + if (peer.IsBusy) + return $"{agent.Name} is busy and cannot answer right now. " + + AgentDirectory.Describe(agent) + + $" Use read_agent_transcript(\"{agent.Name}\") to work out what it has been doing."; + // The loop guard runs before the attempt, since a chain limit is a reason not to ask at all // rather than something the target should be woken to discover. if (AgentCallChain.Reject(agent.Key, agent.Name) is { } rejection) return rejection; - // No busy check here. The peer claims itself atomically and reports whether it could — - // checking first and asking second left a gap where the agent could take a turn in between, - // and produced a second, worse-worded refusal from the far side. One decision, one place; - // this layer only chooses how to say it, because only this layer knows the directory. - var result = await peer.AskAsync(_selfName, question); - if (result.Answered) return $"{agent.Name} replied:\n{result.Text}"; + var delegation = _directory.Delegations.Open( + _selfKey, _selfName, agent.Key, agent.Name, question, DelegationKind.Question); + _start(delegation, peer); - return $"{agent.Name} could not answer ({result.Text}). " + - AgentDirectory.Describe(agent) + - $" Use read_agent_transcript(\"{agent.Name}\") to work out what it has been doing."; + return $"Asked {agent.Name}: {question}\n" + + "It is working out an answer now. The reply will appear in this conversation when it " + + "arrives, and you will have it on your next turn — so carry on with the user rather " + + "than waiting, and do not guess at the answer in the meantime."; } [Description("Reads the recent conversation from another agent's tab, so you can work out what " + diff --git a/src/MandoCode.Desktop/Services/Delegations.cs b/src/MandoCode.Desktop/Services/Delegations.cs index 31eb81a..57306d4 100644 --- a/src/MandoCode.Desktop/Services/Delegations.cs +++ b/src/MandoCode.Desktop/Services/Delegations.cs @@ -2,7 +2,15 @@ public enum DelegationState { Running, Done, Failed } -/// One job one agent handed to another. +/// +/// Why one agent handed work to another. Both travel the identical path — the target takes a real +/// turn either way — so this changes only how the result is WORDED back to the asker. A question +/// answered is not a job finished, and a card reading "finished" for a question asked in passing +/// would suggest work had been done. +/// +public enum DelegationKind { Job, Question } + +/// One piece of work one agent handed to another. public sealed record Delegation( string Id, string FromKey, @@ -13,7 +21,8 @@ public sealed record Delegation( DateTimeOffset StartedAt, DelegationState State = DelegationState.Running, string? Result = null, - DateTimeOffset? FinishedAt = null); + DateTimeOffset? FinishedAt = null, + DelegationKind Kind = DelegationKind.Job); /// /// Outstanding delegations, and the rolling digest each one contributes to its owner's inbox. @@ -30,11 +39,13 @@ public sealed class DelegationRegistry private readonly Dictionary _byId = new(StringComparer.Ordinal); private int _next; - public Delegation Open(string fromKey, string fromName, string toKey, string toName, string task) + public Delegation Open(string fromKey, string fromName, string toKey, string toName, string task, + DelegationKind kind = DelegationKind.Job) { lock (_lock) { - var d = new Delegation($"d{++_next}", fromKey, fromName, toKey, toName, task, DateTimeOffset.Now); + var d = new Delegation($"d{++_next}", fromKey, fromName, toKey, toName, task, + DateTimeOffset.Now, Kind: kind); _byId[d.Id] = d; return d; } @@ -92,17 +103,28 @@ public void Sweep(TimeSpan keepFor) public static InboxMessage Digest(Delegation d, AgentEntry? peer, IReadOnlyList recentCommands) { var age = Age(DateTimeOffset.Now - d.StartedAt); - var lines = new List { $"You asked {d.ToName} to: {d.Task}" }; + var ask = d.Kind == DelegationKind.Question; + var lines = new List + { + ask ? $"You asked {d.ToName}: {d.Task}" : $"You asked {d.ToName} to: {d.Task}" + }; switch (d.State) { case DelegationState.Done: - lines.Add($"FINISHED after {Age(d.FinishedAt - d.StartedAt ?? TimeSpan.Zero)}."); - if (!string.IsNullOrWhiteSpace(d.Result)) lines.Add($"{d.ToName} reported: {d.Result}"); + lines.Add(ask + ? $"ANSWERED after {Age(d.FinishedAt - d.StartedAt ?? TimeSpan.Zero)}." + : $"FINISHED after {Age(d.FinishedAt - d.StartedAt ?? TimeSpan.Zero)}."); + // The reply is carried whole. This is the copy the model reads on its next turn, so + // trimming it here would make the clipped version the only one it ever sees. + if (!string.IsNullOrWhiteSpace(d.Result)) + lines.Add(ask ? $"{d.ToName} replied: {d.Result}" : $"{d.ToName} reported: {d.Result}"); break; case DelegationState.Failed: - lines.Add($"DID NOT FINISH: {d.Result ?? "no reason given"}."); + lines.Add(ask + ? $"DID NOT ANSWER: {d.Result ?? "no reason given"}." + : $"DID NOT FINISH: {d.Result ?? "no reason given"}."); break; default: @@ -111,7 +133,9 @@ public static InboxMessage Digest(Delegation d, AgentEntry? peer, IReadOnlyList< lines.Add($"STOPPED — {d.ToName}'s tab was closed before it finished."); break; } - lines.Add($"Still working ({age} so far)."); + lines.Add(ask + ? $"Still working out an answer ({age} so far)." + : $"Still working ({age} so far)."); if (peer.PlanTotal > 0) lines.Add($"On step {peer.PlanStep} of {peer.PlanTotal}."); if (peer.IsRunningCommand) lines.Add("A command is running right now."); if (recentCommands.Count > 0) @@ -136,13 +160,27 @@ public static InboxMessage Digest(Delegation d, AgentEntry? peer, IReadOnlyList< /// public static string CompletionLine(Delegation d) { + var ask = d.Kind == DelegationKind.Question; var label = Shorten(d.Task, 70); - if (d.State != DelegationState.Done) - return $"✗ {d.ToName} did not finish \"{label}\" — {Shorten(d.Result ?? "no reason given", 160)}"; - var outcome = Shorten(FirstMeaningfulLine(d.Result), 200); - return string.IsNullOrEmpty(outcome) - ? $"✓ {d.ToName} finished \"{label}\"" + if (d.State != DelegationState.Done) + return ask + ? $"✗ {d.ToName} could not answer \"{label}\" — {Shorten(d.Result ?? "no reason given", 160)}" + : $"✗ {d.ToName} did not finish \"{label}\" — {Shorten(d.Result ?? "no reason given", 160)}"; + + // An answer gets far more room than a job's outcome. For a job the interesting thing is + // THAT it finished — the work itself is in the files. For a question the reply IS the + // deliverable, and clipping it to a job's length would send the user to the other agent's + // tab to read two sentences. + var outcome = ask + ? Shorten(d.Result?.Trim() ?? "", 600) + : Shorten(FirstMeaningfulLine(d.Result), 200); + + if (string.IsNullOrEmpty(outcome)) + return ask ? $"↩ {d.ToName} replied to \"{label}\"" : $"✓ {d.ToName} finished \"{label}\""; + + return ask + ? $"↩ {d.ToName} replied to \"{label}\" — {outcome}" : $"✓ {d.ToName} finished \"{label}\" — {outcome}"; } @@ -171,10 +209,13 @@ private static string Shorten(string text, int max) return (space > max / 2 ? cut[..space] : cut).TrimEnd(',', '.', ';', ' ') + "…"; } - private static string Headline(Delegation d) => d.State switch + private static string Headline(Delegation d) => (d.Kind, d.State) switch { - DelegationState.Done => "finished the job you delegated", - DelegationState.Failed => "could not finish the job you delegated", + (DelegationKind.Question, DelegationState.Done) => "answered your question", + (DelegationKind.Question, DelegationState.Failed) => "could not answer your question", + (DelegationKind.Question, _) => "working out an answer for you", + (_, DelegationState.Done) => "finished the job you delegated", + (_, DelegationState.Failed) => "could not finish the job you delegated", _ => "working on the job you delegated", };