diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d84459..8124fa5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,17 @@ for every approved plan. Desktop's version follows the engine generation, so it 0.15.0. ### Added +- **Agents can talk to each other.** Typing `@` now offers the other open agents before project + files, so you can address one by name from another's conversation. An agent can check what another + is doing, read its conversation, ask it a question and get a real answer back, or hand it a whole + job. Handing over a job does not block: your agent replies immediately and stays available while + the other works, tells you how far along it is whenever you ask, and announces the result in the + conversation when it finishes. Progress is summarised from what the other agent is actually doing + — the step it is on, the commands it has run — rather than by copying its conversation across, so + a job that runs for ten minutes costs no more to keep track of than one that runs for ten seconds. + Questions relayed between agents are labelled as such, so an agent always knows whether it is + talking to you or to another agent, and treats what another agent tells it as a claim rather than + a fact. - **Nine themes that imitate a physical medium, not just a colour scheme.** Alongside the existing e-ink and CRT looks, MandoCode Desktop now ships a monochrome amber terminal, a vacuum-fluorescent panel, a vector scope, a passive-matrix LCD, a Solari split-flap board, a cyanotype blueprint, a diff --git a/docs/agent-mentions.md b/docs/agent-mentions.md new file mode 100644 index 0000000..0f5f4be --- /dev/null +++ b/docs/agent-mentions.md @@ -0,0 +1,274 @@ +# Agent mentions — design + +Status: **built**, except `review_agent_work`. See the build order below. + +## What it is + +Typing `@Knuckles` in one agent's input addresses another open agent. The addressed agent can be +asked what it is doing, have its work reviewed, have its transcript read, or — at the far end — be +asked a genuine question that costs it a turn. + +## The insight the design turns on + +The obvious implementation is "pull Knuckles' transcript into Sonic's context." That is the wrong +default, for two reasons: + +1. **It is the expensive option.** A transcript is thousands of tokens, copied into Sonic's window + and paid for on every subsequent turn of Sonic's conversation. Asking Knuckles a question costs + one turn and returns a few hundred tokens of *answer*. +2. **It is the lossy option.** Sonic has to work out which parts of a long transcript matter. + Knuckles already knows. + +So the transcript is the *escalation*, not the starting point. + +The second insight is that **most of what you want does not require waking the other agent at all**: + +| Tool | Wakes the target's model | Works while target is busy | Answers | +|---|---|---|---| +| `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 | + +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. + +## Build order + +Each step is useful alone and complexity rises monotonically. Stopping after any of them leaves a +coherent feature. + +1. `agent_status` — **built**, as `list_agents` and `get_agent_status` +2. `review_agent_work` — still proposed +3. `read_agent_transcript` — **built** +4. `ask_agent` — **built** + +## The tools + +Registered per agent through `AIService.SetHostTools`, the same seam the browser tools use +(`AgentSession` already calls it with `AIFunctionFactory.Create(...)` for each preview tool). The +host owns them, so they can see every open session while each agent's own file tools stay bounded to +its own root. + +### 1. `agent_status(name)` + +Returns host-observable facts. No model call anywhere. + +Everything needed already exists: + +- `BusyStateService.IsBusy` — working or idle +- `ChatController.PlanProgressChanged` — already emits `CurrentStep` / `TotalSteps` +- `AgentCommandLog.IsRunning` and its scrollback — the commands that agent has run +- the session's project root, model and title + +This is the tool that makes "have you finished the X task?" answerable *at the moment you would +actually ask it* — which is while the other agent is still working. An earlier draft of this design +had the busy case refuse; that would have failed the feature's most common question. + +### 2. `review_agent_work(name)` + +Returns the target's working-tree diff so the *asking* agent can form its own judgment. + +Self-assessment is the weakest form of review — asking an agent whether its own work is good is +nearly worthless. Handing the reviewer the artifact is a real review. + +The artifact already exists: `GitQuickStatus` yields `GitChangeEntry` (path + change kind) and +`GitFileDiff` (parsed diff lines, with a `Truncated` flag for oversized diffs). Pair it with the +target's `AgentCommandLog` and the reviewer sees both what changed and what was run to produce it. + +**Torn reads.** A working tree that is actively being written gives a half-finished picture. The +mitigation is already in hand: this tool checks `IsBusy` first and prefixes its result with a +warning when the target is mid-turn, rather than silently returning a diff that is about to change. + +### 3. `read_agent_transcript(name, ...)` + +The escalation. Expensive and explicit, reached for when an answer or a diff was not enough. +`IAiService.ExportHistoryJson()` already serialises a conversation. + +The model chooses between this and `ask_agent` on its own — describing both honestly is what +implements "pull the transcript only when more context is needed." There is no condition to detect. + +### 4. `ask_agent(name, question)` + +Runs a real turn on the target's model and returns its answer. + +`AIService.ChatStreamWithHostInstructionAsync` is the right primitive. Its guidance is carried as +"a real, transient system-role message… available for this turn and its continuations but removed +afterward, so it cannot masquerade as user-authored text or affect later turns" — exactly what a +delegated turn needs. Knuckles is told "Sonic is asking you this" for one turn only, with no +contamination of its own later conversation. + +Open problems, all of which belong to this tool alone: + +- **Loops — settled: `AgentCallChain`, an AsyncLocal chain with two limits.** A key set catches a + true cycle (an agent already in this chain being asked again); a depth cap of 2 catches a chain + that never repeats anyone but keeps going. AsyncLocal rather than a field, because the chain + belongs to one call sequence — two conversations asking questions at once must not consume each + other's budget. +- **Concurrency — settled: refuse, and route the caller to the read tools.** If the target is + 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. +- **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 + is for. + + The containment is that the target's own approval gates still stand, and they raise their dialogs + in the target's own tab, where the user can see who is being asked to do what. The host + instruction that frames the turn ("another agent is asking you this") is a *framing, not a + sandbox* — a model can wander past prompt-level guidance, so it is not relied on for safety. + +## The `@` picker + +`ChatTabView.Input.cs` already implements `@` for project files: it walks back from the caret to the +token start, and if the token opens with `@` it filters through `FileAutocompleteProvider` and calls +`ShowSuggestions(SuggestMode.File, ...)`. + +Agents join that same picker and **rank above files**. Rationale: + +- Open agents are a small, closed, known set; project files are thousands. A short list on top costs + the file case almost nothing. +- Callsigns are capitalised single words, so genuine collisions with real filenames are rare, and + the picker disambiguates the rare ones visually. +- `@` already means "a participant" to anyone who has used Slack. A second sigil would be a thing to + learn for no benefit. + +Implementation notes: + +- A new `SuggestMode.Agent`, because `AcceptSuggestion` branches on the mode and an agent mention + substitutes differently from a file path — no trailing `/` drill-in behaviour, and the accepted + text should be the callsign, not a path. +- The agent's own tab must be excluded from its own picker. +- Suggestion rows want a distinguishing glyph and a subtitle (folder name, or busy/idle), so an + agent row never reads as a file row. +- **The host must also learn about mentions, not just the picker.** An earlier draft of this doc + claimed the picker was an affordance and no host-side parsing was needed. That was wrong, and + testing found it immediately: `ChatController.ProcessFileReferences` expands every `@token` at + send time and warns `Couldn't find the referenced file or folder: Ninja` when the lookup misses. + A mention never reached the model at all. Agent names must be resolved *before* the file lookup + and skipped by it. +- Agents win over files on a name clash. A callsign is a deliberate act of addressing someone; a + same-named file is a coincidence, and that file stays reachable by any path carrying a separator + or an extension. + +## Who am I talking to + +A receiving agent must be able to tell a relayed question from something the user typed, and it must +be able to tell *structurally* rather than by reading the content. + +The first implementation put the attribution only in the host instruction. That was not enough, and +testing showed why within minutes: the instruction is transient by design — the engine removes it +once the turn ends — so the peer's question stayed in the receiving agent's history as an ordinary +user turn. Asked afterwards who it had been talking to, the agent answered that the last message +"claimed" to be from another agent. It was reasoning from content, because content was all it had. + +Worse, one agent told another "the person you're talking to is Mando", and the receiver had no way +to weigh that. A claim inside a relayed message had the same standing as a fact. + +So: + +- **Every agent-to-agent message is wrapped in a host-applied envelope** (`PeerMessageEnvelope`) + naming the sender and stating it is not from the user. This is part of the message, so it persists + in history rather than evaporating with the turn. +- **A message without an envelope is from the user.** That is the rule the framing states, and it is + the only rule needed, because the host is the only thing that can add one. +- **`Wrap` strips any envelope already present in the payload.** Without that, "only MandoCode adds + this" would be a claim the code did not keep — one agent could relay a message that appeared to + come from a third, and an agent innocently quoting the marker while discussing this feature would + produce the same confusion. +- **Claims inside a relayed message are that agent's assertions, not facts.** The framing says so + explicitly, including claims about who the user is. + +## Visibility + +An exchange between agents appears in **both** transcripts — the target's tab showing that it was +asked, and by whom. That is the audit trail, and it also makes the feature legible: you can watch +your agents talk instead of wondering what they said. + +`agent_status`, `review_agent_work` and `read_agent_transcript` are reads and need no entry in the +target's transcript; `ask_agent` produces a real turn there and must be attributed. + +## 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. + +**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` +already uses for imported snapshots. Agents are turn-based, so this is the only moment an idle +agent can take delivery of anything. + +**Progress is a rolling digest, not an event log.** This is the load-bearing decision. A delegation +posts under ONE inbox id for its whole life, so each report REPLACES the last: a job that runs for +ten minutes costs the same context as one that runs for ten seconds. An append-only feed would grow +with the other agent's work and be paid for on every subsequent turn — the transcript-copying +problem arriving in instalments. + +**The digest is assembled, never written by a model.** Every field comes from state the host already +keeps: the turn gate, plan progress, `AgentCommandLog`, the directory entry. So keeping it current +costs string formatting rather than a turn, it is always accurate, and it cannot invent progress. A +model-written précis would cost a call per update and could report a job as nearly done because it +read that way. + +**Notification and knowing are separate, and each is cheap.** A finished job appends a card to the +*delegating* agent's transcript — that is the "tell me when it's done", and it costs no model turn +at all. The agent itself learns from its inbox on its next turn. Neither requires waking anything. + +Rejected along the way: + +- **A publish/subscribe broker.** It answers "who gets the message", but the real blocker is that an + idle agent cannot act on one — so a broker would sit on top of the same two delivery mechanisms + and leave the original problem intact. The routing is also already known: A asked B. +- **Streaming B's actions to A.** The progressive idea was right; the delivery was not. Raw actions + are a firehose, and "A unsubscribes when it knows enough" cannot work — A only decides while + running, so the feed would accumulate unbounded until the user next happened to speak to it. +- **Queueing a job for a busy agent.** It would report work as accepted while nothing had started. + Refusing names the reason and the current state instead. + +## Deliberately out of scope + +- Cross-agent *delegation* ("Knuckles, go fix the auth module"). Blocked on the approval question + above, and worth having mentions in hand before deciding it. +- Mentioning a closed agent or a saved session. Snapshots already cover recovering an old + conversation as context. +- Agents mentioning each other unprompted. Every cross-agent call in this design begins with + something the user typed. + +## Future: reaching Desktop agents from the CLI + +Investigated, deliberately not built. Recorded because the findings are the expensive part. + +- **`read_agent_transcript` is already cross-process.** It reads `ConversationLog.Load(key)` from + disk, not from memory, so a separate process could read a Desktop agent's conversation today if it + knew the key. +- **Discovery is the only missing piece for a read-only bridge.** `RefreshAgentDirectory` already + runs on every tab-strip refresh; writing that snapshot to a file would give a CLI `@`-completion, + status, and transcripts with no IPC and no protocol. It needs a PID and a heartbeat, or the CLI + would confidently list agents belonging to a Desktop that has since exited. The two apps also use + different roots today (`LocalApplicationData/MandoCode.Desktop` versus `~/.mandocode`), which is an + agreement rather than an obstacle. +- **Agent Framework does not hand you remoting, but it does not fight it.** The packages in use + (`Microsoft.Agents.AI` / `.Workflows`) expose no remote, host, proxy or transport types — the + surface is entirely in-process. What they *do* expose is `AIAgent.DeserializeSessionAsync` with a + serializable `AgentSession`, so conversation state already has a wire format; and + `DelegatingAIAgent`, which is exactly the seam for a proxy that forwards `RunCoreAsync` over IPC. + A remote agent would satisfy `IAgentPeer` and none of the four tools would know the difference. +- **Every hard problem here is product-shaped, not framework-shaped.** Where does an approval dialog + appear when the CLI makes a Desktop agent write a file? How does a local endpoint prove the caller + is the user rather than any process on the machine? Where does a completion go when the CLI has + exited mid-job? And the addressing is asymmetric: Desktop has N named agents, the CLI is one + unnamed conversation. A transport answers none of these. + +## The one boundary this widens + +Every agent's file access is bounded to its own `ProjectRootAccessor` by design. +`review_agent_work` deliberately reaches past that so the reviewer can see the target's folder. It +is read-only and scoped to another *open agent's* root, never to arbitrary paths — but it is a real +widening of what an agent can see, and it should be a decision rather than a discovery. If both +agents share a root, nothing is crossed at all. diff --git a/src/MandoCode.Desktop.Tests/AgentAskTests.cs b/src/MandoCode.Desktop.Tests/AgentAskTests.cs new file mode 100644 index 0000000..739ce62 --- /dev/null +++ b/src/MandoCode.Desktop.Tests/AgentAskTests.cs @@ -0,0 +1,311 @@ +using MandoCode.Desktop.Services; +using Xunit; + +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. +/// +public class AgentAskTests +{ + private const string Self = "self-key"; + + private sealed class FakePeer : IAgentPeer + { + public FakePeer(string key, string answer = "done both", bool busy = false) + { + Key = key; Answer = answer; IsBusy = busy; + } + public string Key { get; } + public string Answer { get; } + public bool IsBusy { get; set; } + public int Asked { get; private set; } + public string? LastAskedBy { get; private set; } + public string? LastQuestion { get; private set; } + + public Task AskAsync(string askedBy, string question, CancellationToken cancellationToken = default) + { + Asked++; LastAskedBy = askedBy; LastQuestion = question; + // Mirrors the real peer: it claims itself and reports back, rather than the caller + // deciding from a flag it read a moment earlier. + return Task.FromResult(IsBusy ? PeerAnswer.Busy() : PeerAnswer.Ok(Answer)); + } + } + + 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( + 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); + } + + [Fact] + public async Task DelegatingReturnsImmediatelyWithoutWaitingForTheJob() + { + // The point of the whole feature: the delegating agent's turn ends at once, so the user is + // not locked out of it while a long job runs. + var dir = new AgentDirectory(); + dir.Replace(new[] { Entry("k1", "Ninja"), Entry(Self, "Sonic") }); + var peer = new FakePeer("k1"); + dir.RegisterPeer(peer); + Delegation? started = null; + var tools = new AgentDirectoryTools(dir, Self, () => "Sonic", (d, _) => started = d); + + var result = tools.DelegateToAgent("Ninja", "build the marketing site"); + + Assert.NotNull(started); + Assert.Equal("build the marketing site", started!.Task); + Assert.Equal(0, peer.Asked); // nothing awaited here + Assert.Contains("You will be told when it finishes", result); + await Task.CompletedTask; + } + + [Fact] + public async Task ABusyAgentIsNotGivenAJobToSitOn() + { + // Queuing would tell the user their work was accepted while nothing had started, and hide + // it behind work of unknown length. + var dir = new AgentDirectory(); + dir.Replace(new[] { Entry("k1", "Ninja", busy: true, step: 4, total: 9), Entry(Self, "Sonic") }); + dir.RegisterPeer(new FakePeer("k1", busy: true)); + var startedAnything = false; + var tools = new AgentDirectoryTools(dir, Self, () => "Sonic", (_, _) => startedAnything = true); + + var result = tools.DelegateToAgent("Ninja", "build it"); + + Assert.False(startedAnything); + Assert.Contains("busy", result); + Assert.Contains("step 4 of 9", result); + await Task.CompletedTask; + } + + [Fact] + public async Task CheckingDelegationsReportsEachJobWithoutAskingAnyone() + { + // This is what answers "how's the site coming?" while the other agent is mid-build — no + // turn on either side, and it works precisely because the target is busy. + var dir = new AgentDirectory(); + dir.Replace(new[] { Entry("k1", "Ninja", busy: true, step: 2, total: 5), Entry(Self, "Sonic") }); + dir.RegisterPeer(new FakePeer("k1")); + var tools = new AgentDirectoryTools(dir, Self, () => "Sonic", (_, _) => { }); + + tools.DelegateToAgent("Ninja", "build the site"); + var status = tools.CheckDelegations(); + + Assert.Contains("build the site", status); + Assert.Contains("step 2 of 5", status); + await Task.CompletedTask; + } + + [Fact] + public async Task WithNothingDelegatedTheReportSaysSoPlainly() + { + var (tools, _, _) = Setup(); + Assert.Contains("not handed any work", tools.CheckDelegations()); + await Task.CompletedTask; + } + + [Fact] + public async Task AnIdleAgentIsAskedAndItsAnswerComesBack() + { + var (tools, peer, _) = Setup(answer: "yes, both finished"); + + var result = await tools.AskAgent("Ninja", "have you finished the X and Y tasks?"); + + 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); + } + + [Fact] + public async Task TheAnswerIsAttributedToTheAgentDoingTheAsking() + { + // The target shows this in its own transcript. Unattributed, a question would read as + // something the user typed. + var (tools, peer, _) = Setup(); + + await tools.AskAgent("Ninja", "status?"); + + Assert.Equal("Sonic", peer.LastAskedBy); + } + + [Fact] + public async Task ABusyAgentDeclinesAndTheCallerIsPointedAtTheReadTools() + { + // 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); + + var result = await tools.AskAgent("Ninja", "done yet?"); + + Assert.Equal(1, peer.Asked); + Assert.Contains("could not answer", result); + Assert.Contains("step 4 of 9", result); + Assert.Contains("read_agent_transcript", result); + } + + [Fact] + public async Task AnAgentThatBecomesBusyBetweenTheHintAndTheAskStillDeclinesCleanly() + { + // 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 + + var result = await tools.AskAgent("Ninja", "done yet?"); + + Assert.Contains("could not answer", result); + Assert.Contains("read_agent_transcript", result); + } + + [Fact] + public async Task AnUnknownAgentNamesTheOnesThatExist() + { + var (tools, peer, _) = Setup(); + + var result = await tools.AskAgent("Knuckles", "hello?"); + + Assert.Equal(0, peer.Asked); + Assert.Contains("Ninja", result); + } + + [Fact] + public async Task AskingYourselfIsRefused() + { + var (tools, peer, _) = Setup(); + Assert.Contains("is you", await tools.AskAgent("Sonic", "what am I doing?")); + Assert.Equal(0, peer.Asked); + } + + [Fact] + 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(); + dir.RemovePeer("k1"); + + var result = await tools.AskAgent("Ninja", "still there?"); + + Assert.Equal(0, peer.Asked); + Assert.Contains("closed", result); + } + + [Fact] + 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(); + + using (AgentCallChain.Enter("k1")) + { + var result = await tools.AskAgent("Ninja", "and you?"); + Assert.Equal(0, peer.Asked); + Assert.Contains("loop", result); + } + } + + [Fact] + 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(); + + using (AgentCallChain.Enter("other-1")) + using (AgentCallChain.Enter("other-2")) + { + var result = await tools.AskAgent("Ninja", "one more?"); + Assert.Equal(0, peer.Asked); + Assert.Contains("limit", result); + } + } + + [Fact] + public async Task TheGuardSurvivesTheAwaitsAndThreadHopsOfARealCall() + { + // The chain is carried in an AsyncLocal, and the check happens deep inside the engine's + // tool-invocation machinery rather than next to the Enter() that set it. If execution + // context did not flow across those boundaries the guard would stop guarding SILENTLY — + // no exception, no failing test, just two agents talking until the budget is gone. + // + // 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(); + + using (AgentCallChain.Enter("k1")) + { + await Task.Yield(); + await Task.Run(async () => + { + await Task.Delay(1).ConfigureAwait(false); + var result = await tools.AskAgent("Ninja", "still looping?").ConfigureAwait(false); + Assert.Contains("loop", result); + }).ConfigureAwait(false); + } + + Assert.Equal(0, peer.Asked); + } + + [Fact] + 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 blocked = Task.Run(async () => + { + using (AgentCallChain.Enter("other-1")) + using (AgentCallChain.Enter("other-2")) + { + await Task.Delay(5).ConfigureAwait(false); + return await tools.AskAgent("Ninja", "deep chain").ConfigureAwait(false); + } + }); + + var allowed = Task.Run(async () => + { + await Task.Delay(5).ConfigureAwait(false); + return await tools.AskAgent("Ninja", "fresh chain").ConfigureAwait(false); + }); + + 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); + } + + [Fact] + 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(); + + using (AgentCallChain.Enter("other-1")) + using (AgentCallChain.Enter("other-2")) { } + + var result = await tools.AskAgent("Ninja", "now?"); + + Assert.Equal(1, peer.Asked); + Assert.Contains("Ninja replied", result); + } +} diff --git a/src/MandoCode.Desktop.Tests/AgentDirectoryTests.cs b/src/MandoCode.Desktop.Tests/AgentDirectoryTests.cs new file mode 100644 index 0000000..b36ceeb --- /dev/null +++ b/src/MandoCode.Desktop.Tests/AgentDirectoryTests.cs @@ -0,0 +1,104 @@ +using MandoCode.Desktop.Services; +using Xunit; + +namespace MandoCode.Desktop.Tests; + +/// +/// The register behind '@' mentions. The behaviour that matters is resolution: a mention is the user +/// naming a specific colleague, so guessing wrong is worse than not resolving at all. +/// +public class AgentDirectoryTests +{ + private static AgentEntry Agent(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 AgentDirectory With(params AgentEntry[] agents) + { + var d = new AgentDirectory(); + d.Replace(agents); + return d; + } + + [Fact] + public void ResolvesACallsignRegardlessOfCase() + { + var d = With(Agent("k1", "Ninja")); + Assert.Equal("k1", d.Resolve("ninja")?.Key); + Assert.Equal("k1", d.Resolve("NINJA")?.Key); + } + + [Fact] + public void AnExactNameIsNeverShadowedByALongerOne() + { + // "Ninja" and "NinjaTwo" both start with "Ninja". Addressing Ninja must reach Ninja. + var d = With(Agent("k1", "Ninja"), Agent("k2", "NinjaTwo")); + Assert.Equal("k1", d.Resolve("Ninja")?.Key); + } + + [Fact] + public void AnAmbiguousPrefixResolvesToNothing() + { + // Two candidates and no exact match: refusing is right. Picking one would silently send a + // question to an agent the user did not name. + var d = With(Agent("k1", "Ninja"), Agent("k2", "Nitro")); + Assert.Null(d.Resolve("Ni")); + } + + [Fact] + public void AnUnambiguousPrefixResolves() + { + var d = With(Agent("k1", "Ninja"), Agent("k2", "Falchion")); + Assert.Equal("k1", d.Resolve("Nin")?.Key); + } + + [Fact] + public void AnUnknownNameResolvesToNothing() + { + Assert.Null(With(Agent("k1", "Ninja")).Resolve("Sonic")); + Assert.Null(With(Agent("k1", "Ninja")).Resolve("")); + } + + [Fact] + public void ThePickerExcludesTheAgentDoingTheTyping() + { + // An agent mentioning itself is never what was meant, and offering it invites the confusion. + var d = With(Agent("self", "Ninja"), Agent("other", "Falchion")); + var matches = d.Match("", excludeKey: "self"); + Assert.Equal(new[] { "Falchion" }, matches.Select(a => a.Name)); + } + + [Fact] + public void ThePickerRanksPrefixMatchesFirst() + { + // Typing "ni" means you are probably reaching for Ninja, not for the agent that merely + // contains those letters. + var d = With(Agent("k1", "Hornight"), Agent("k2", "Ninja")); + var matches = d.Match("ni", excludeKey: null); + Assert.Equal(new[] { "Ninja", "Hornight" }, matches.Select(a => a.Name)); + } + + [Fact] + public void DescribeLeadsWithWhetherTheAgentIsActuallyWorking() + { + // This is the sentence that answers "have you finished yet", so busy versus idle has to be + // unmissable rather than inferred from surrounding detail. + var busy = AgentDirectory.Describe(Agent("k1", "Ninja", busy: true, step: 3, total: 7)); + var idle = AgentDirectory.Describe(Agent("k2", "Falchion")); + + Assert.Contains("WORKING", busy); + Assert.Contains("step 3 of 7", busy); + Assert.Contains("IDLE", idle); + Assert.DoesNotContain("step", idle); + } + + [Fact] + public void SnapshotsDoNotChangeUnderTheCaller() + { + // Tools read the register on model-loop threads while the UI republishes it. A caller that + // took a list must keep the list it took. + var d = With(Agent("k1", "Ninja")); + var taken = d.All; + d.Replace(new[] { Agent("k2", "Falchion") }); + Assert.Equal(new[] { "Ninja" }, taken.Select(a => a.Name)); + } +} diff --git a/src/MandoCode.Desktop.Tests/AgentDirectoryToolsTests.cs b/src/MandoCode.Desktop.Tests/AgentDirectoryToolsTests.cs new file mode 100644 index 0000000..8d9a04d --- /dev/null +++ b/src/MandoCode.Desktop.Tests/AgentDirectoryToolsTests.cs @@ -0,0 +1,98 @@ +using MandoCode.Desktop.Services; +using Xunit; + +namespace MandoCode.Desktop.Tests; + +/// +/// What the model actually receives when it asks about another agent. These are read tools, so the +/// bar is that a wrong or missing name produces something the model can act on rather than a dead +/// end — a failed lookup should tell it who IS open. +/// +public class AgentDirectoryToolsTests +{ + private const string Self = "self-key"; + + private static AgentEntry Agent(string key, string name, bool busy = false, bool cmd = false, + int step = 0, int total = 0) => + new(key, name, $@"C:\src\{name}", name.ToLowerInvariant(), "qwen3:8b", busy, cmd, step, total); + + private static (AgentDirectoryTools Tools, AgentDirectory Dir) Setup(params AgentEntry[] agents) + { + var d = new AgentDirectory(); + d.Replace(agents.Append(Agent(Self, "Sonic"))); + return (new AgentDirectoryTools(d, Self), d); + } + + [Fact] + public void StatusReportsWorkingWithPlanPosition() + { + var (tools, _) = Setup(Agent("k1", "Ninja", busy: true, cmd: true, step: 5, total: 6)); + + var status = tools.GetAgentStatus("Ninja"); + + Assert.Contains("WORKING", status); + Assert.Contains("step 5 of 6", status); + Assert.Contains("shell command is running", status); + } + + [Fact] + public void StatusAnswersTheQuestionEvenWhileTheAgentIsMidTurn() + { + // The whole point: "have you finished X?" is asked precisely when the answer might be no. + // This tool reads state, so a busy agent is an answer rather than a refusal. + var (tools, _) = Setup(Agent("k1", "Ninja", busy: true)); + + var status = tools.GetAgentStatus("Ninja"); + + Assert.DoesNotContain("busy", status, StringComparison.OrdinalIgnoreCase); + Assert.Contains("WORKING", status); + } + + [Fact] + public void AnUnknownNameNamesTheAgentsThatDoExist() + { + // A bare "not found" leaves the model guessing. Listing the real names lets it recover in + // the same turn, which is usually a typo or a half-remembered callsign. + var (tools, _) = Setup(Agent("k1", "Ninja"), Agent("k2", "Falchion")); + + var status = tools.GetAgentStatus("Knuckles"); + + Assert.Contains("No agent named \"Knuckles\"", status); + Assert.Contains("Ninja", status); + Assert.Contains("Falchion", status); + } + + [Fact] + public void AskingAboutYourselfSaysSoRatherThanReportingNothing() + { + var (tools, _) = Setup(); + + var status = tools.GetAgentStatus("Sonic"); + + Assert.Contains("this agent", status); + } + + [Fact] + public void ListingExcludesTheAskerAndMarksWhoIsWorking() + { + var (tools, _) = Setup(Agent("k1", "Ninja", busy: true), Agent("k2", "Falchion")); + + var list = tools.ListAgents(); + + Assert.DoesNotContain("Sonic", list); + Assert.Contains("Ninja", list); + Assert.Contains("(working)", list); + Assert.Contains("(idle)", list); + } + + [Fact] + public void ASoleAgentIsToldItIsAlone() + { + // Distinct from "not found": there is nobody to mention, so the model should stop looking + // rather than try another spelling. + var (tools, _) = Setup(); + + Assert.Contains("No other agents are open", tools.ListAgents()); + Assert.Contains("no other agents open at all", tools.GetAgentStatus("Ninja")); + } +} diff --git a/src/MandoCode.Desktop.Tests/ConversationRoleTests.cs b/src/MandoCode.Desktop.Tests/ConversationRoleTests.cs new file mode 100644 index 0000000..7a407c8 --- /dev/null +++ b/src/MandoCode.Desktop.Tests/ConversationRoleTests.cs @@ -0,0 +1,76 @@ +using MandoCode.Desktop.Services; +using Xunit; + +namespace MandoCode.Desktop.Tests; + +/// +/// Who said what, after the app has been closed and reopened. +/// +/// The bug these cover: a question from another agent was never written to the conversation +/// log at all. The log held the ANSWER with nothing before it, so a restored session re-briefed the +/// model with a dangling reply, and read_agent_transcript showed answers with nothing that had +/// prompted them. +/// +public class ConversationRoleTests +{ + [Fact] + public void AnAgentTurnIsLabelledAsAnAgentNotAsTheUser() + { + // Replaying a relayed question as "User:" after a restart is exactly the confusion the + // envelope exists to prevent — and a restart is the one place a transient host instruction + // could never reach. + Assert.Equal("Another agent", ConversationLog.RoleLabel(ConversationLog.AgentRole)); + Assert.Equal("User", ConversationLog.RoleLabel("u")); + Assert.Equal("Assistant", ConversationLog.RoleLabel("a")); + } + + [Fact] + public void AnUnknownRoleFallsBackToAssistantRatherThanVanishing() + { + // Logs written by an older build carry roles this one has never seen. Rendering something + // is better than dropping a turn out of the replay. + Assert.Equal("Assistant", ConversationLog.RoleLabel("?")); + Assert.Equal("Assistant", ConversationLog.RoleLabel("")); + } + + [Fact] + public void ARelayedQuestionSurvivesARoundTripThroughTheLog() + { + // The full path a restart takes: wrap, persist, reload, relabel. The envelope has to still + // be legible at the end of it. + var key = "roundtrip-" + Guid.NewGuid().ToString("N"); + try + { + var envelope = PeerMessageEnvelope.Wrap("Ninja", "have you finished the X task?"); + ConversationLog.Append(key, ConversationLog.AgentRole, envelope); + ConversationLog.Append(key, "a", "yes, both are done"); + + var turns = ConversationLog.Load(key); + + Assert.Equal(2, turns.Count); + Assert.Equal(ConversationLog.AgentRole, turns[0].R); + Assert.Contains("sent by the agent \"Ninja\"", turns[0].T); + Assert.Contains("NOT from the user", turns[0].T); + Assert.Equal("Another agent", ConversationLog.RoleLabel(turns[0].R)); + } + finally { ConversationLog.Delete(key); } + } + + [Fact] + public void AnAnswerIsNoLongerLeftWithoutTheQuestionThatPromptedIt() + { + // The shape of the original bug, asserted directly: an assistant turn with no preceding + // turn is what a restored session used to re-brief the model with. + var key = "paired-" + Guid.NewGuid().ToString("N"); + try + { + ConversationLog.Append(key, ConversationLog.AgentRole, PeerMessageEnvelope.Wrap("Ninja", "status?")); + ConversationLog.Append(key, "a", "still building"); + + var turns = ConversationLog.Load(key); + + Assert.NotEqual("a", turns[0].R); // something precedes the answer + } + finally { ConversationLog.Delete(key); } + } +} diff --git a/src/MandoCode.Desktop.Tests/DelegationTests.cs b/src/MandoCode.Desktop.Tests/DelegationTests.cs new file mode 100644 index 0000000..a042d70 --- /dev/null +++ b/src/MandoCode.Desktop.Tests/DelegationTests.cs @@ -0,0 +1,253 @@ +using MandoCode.Desktop.Services; +using Xunit; + +namespace MandoCode.Desktop.Tests; + +/// +/// Handing a job to another agent without blocking on it. The design rests on one property — the +/// progress report is O(1) however long the job runs — so that is what most of these check. +/// +public class DelegationTests +{ + private static AgentEntry Entry(string key, string name, bool busy = true, bool cmd = false, + int step = 0, int total = 0) => + new(key, name, $@"C:\src\{name}", name.ToLowerInvariant(), "qwen3:8b", busy, cmd, step, total); + + private static (DelegationRegistry Reg, Delegation D) Open() => + (new DelegationRegistry(), new DelegationRegistry().Open("a", "Sonic", "b", "Ninja", "build the site")); + + // ---- The inbox -------------------------------------------------------------- + + [Fact] + public void PostingTheSameIdReplacesRatherThanAccumulates() + { + // The whole cost argument. Ten minutes of progress reports must cost the same as one. + var inbox = new AgentInbox(); + + for (int i = 1; i <= 200; i++) + inbox.Post(new InboxMessage("delegation:d1", "Ninja — working", $"step {i} of 200", DateTimeOffset.Now)); + + var messages = inbox.Peek(); + Assert.Single(messages); + Assert.Contains("step 200 of 200", messages[0].Body); + } + + [Fact] + public void DistinctMessagesAreCappedSoAProducerCannotGrowItWithoutBound() + { + var inbox = new AgentInbox(); + for (int i = 0; i < AgentInbox.MaxMessages + 10; i++) + inbox.Post(new InboxMessage($"m{i}", "s", "b", DateTimeOffset.Now)); + + Assert.Equal(AgentInbox.MaxMessages, inbox.Peek().Count); + // Oldest dropped first: a stale report matters less than the newest event. + Assert.DoesNotContain(inbox.Peek(), m => m.Id == "m0"); + Assert.Contains(inbox.Peek(), m => m.Id == $"m{AgentInbox.MaxMessages + 9}"); + } + + [Fact] + public void DrainingEmptiesTheMailboxSoNothingIsDeliveredTwice() + { + var inbox = new AgentInbox(); + inbox.Post(new InboxMessage("m1", "s", "b", DateTimeOffset.Now)); + + Assert.Single(inbox.Drain()); + Assert.Empty(inbox.Drain()); + Assert.False(inbox.HasMessages); + } + + [Fact] + public void TheDeliveredTextIsFramedAsSomethingThatHappenedNotSomethingSaid() + { + // Otherwise a delegation report reads as the user speaking, which is the same confusion the + // agent-message envelope exists to prevent. + var text = AgentInbox.Format(new[] + { + new InboxMessage("m1", "Ninja — finished", "the site is built", DateTimeOffset.Now) + }); + + Assert.Contains("While you were away", text); + Assert.Contains("Background you now have", text); + Assert.Contains("the site is built", text); + } + + [Fact] + public void AnEmptyMailboxFormatsToNothingAtAll() + { + // A turn with no news must not carry an empty header into the model's context. + Assert.Equal("", AgentInbox.Format(Array.Empty())); + } + + // ---- The digest ------------------------------------------------------------- + + [Fact] + public void AProgressDigestSaysWhatTheOtherAgentIsActuallyDoing() + { + var reg = new DelegationRegistry(); + var d = reg.Open("a", "Sonic", "b", "Ninja", "build the marketing site"); + + var digest = DelegationRegistry.Digest( + d, Entry("b", "Ninja", cmd: true, step: 4, total: 7), new[] { "npm install", "npm run build" }); + + Assert.Contains("build the marketing site", digest.Body); + Assert.Contains("step 4 of 7", digest.Body); + Assert.Contains("command is running", digest.Body); + Assert.Contains("npm run build", digest.Body); + } + + [Fact] + public void EveryProgressDigestSharesOneInboxIdSoItOverwrites() + { + var reg = new DelegationRegistry(); + var d = reg.Open("a", "Sonic", "b", "Ninja", "build it"); + + var early = DelegationRegistry.Digest(d, Entry("b", "Ninja", step: 1, total: 7), Array.Empty()); + var later = DelegationRegistry.Digest(d, Entry("b", "Ninja", step: 6, total: 7), Array.Empty()); + + Assert.Equal(early.Id, later.Id); + } + + [Fact] + public void AFinishedDigestCarriesTheResult() + { + var reg = new DelegationRegistry(); + var d = reg.Open("a", "Sonic", "b", "Ninja", "build it"); + var done = reg.Complete(d.Id, DelegationState.Done, "deployed to /dist")!; + + var digest = DelegationRegistry.Digest(done, Entry("b", "Ninja", busy: false), Array.Empty()); + + Assert.Contains("FINISHED", digest.Body); + Assert.Contains("deployed to /dist", digest.Body); + } + + [Fact] + public void AFailedJobSaysSoRatherThanReadingAsStillWorking() + { + var reg = new DelegationRegistry(); + var d = reg.Open("a", "Sonic", "b", "Ninja", "build it"); + var failed = reg.Complete(d.Id, DelegationState.Failed, "busy")!; + + var digest = DelegationRegistry.Digest(failed, Entry("b", "Ninja"), Array.Empty()); + + Assert.Contains("DID NOT FINISH", digest.Body); + } + + [Fact] + public void AJobWhoseAgentClosedIsReportedStoppedNotRunning() + { + // A tab can close mid-job. Reporting it as "still working" would leave the delegating agent + // waiting on something that can never finish. + var reg = new DelegationRegistry(); + var d = reg.Open("a", "Sonic", "b", "Ninja", "build it"); + + var digest = DelegationRegistry.Digest(d, peer: null, Array.Empty()); + + Assert.Contains("STOPPED", digest.Body); + Assert.Contains("tab was closed", digest.Body); + } + + // ---- The completion card ---------------------------------------------------- + + [Fact] + public void TheCardLeadsWithWhatHappenedNotWithTheBriefItWasGiven() + { + // Observed live: the card echoed back a paragraph of instructions the user had just watched + // their agent compose, and never said what the job actually produced. + var reg = new DelegationRegistry(); + var brief = "Mando would like you to restyle the Xbox Series X25 webpage to be Halo-themed. " + + "Please update the page's visual design to evoke the Halo franchise - think the " + + "Halo green/olive palette, Master Chief / Spartan aesthetic, sci-fi military styling."; + var d = reg.Open("a", "Falchion", "b", "Ninja", brief); + var done = reg.Complete(d.Id, DelegationState.Done, + "Done - the Halo restyle is complete and live in the preview, with a UNSC top bar.")!; + + var card = DelegationRegistry.CompletionLine(done); + + Assert.Contains("Halo restyle is complete", card); // the result is there + Assert.DoesNotContain("Master Chief", card); // the brief is not replayed + Assert.True(card.Length < 300, $"card is {card.Length} chars - too long to scan"); + } + + [Fact] + public void ALongBriefSurvivesOnlyAsAShortLabel() + { + // Enough to tell two outstanding jobs apart; not enough to be a wall of text. + var reg = new DelegationRegistry(); + var d = reg.Open("a", "Falchion", "b", "Ninja", new string('x', 40) + " " + new string('y', 200)); + var done = reg.Complete(d.Id, DelegationState.Done, "finished it")!; + + var card = DelegationRegistry.CompletionLine(done); + + Assert.Contains("…", card); + Assert.DoesNotContain(new string('y', 200), card); + } + + [Fact] + public void APreambleLikeDoneIsSkippedInFavourOfTheRealSentence() + { + // Models routinely open with a bare acknowledgement. A card showing only that says nothing. + var reg = new DelegationRegistry(); + var d = reg.Open("a", "Falchion", "b", "Ninja", "build it"); + var done = reg.Complete(d.Id, DelegationState.Done, + "Done.\n\nThe page now uses an olive palette with a UNSC dossier bar across the top.")!; + + var card = DelegationRegistry.CompletionLine(done); + + Assert.Contains("olive palette", card); + } + + [Fact] + public void AFailedCardCarriesTheReasonRatherThanTheBrief() + { + var reg = new DelegationRegistry(); + var d = reg.Open("a", "Falchion", "b", "Ninja", "build the site"); + var failed = reg.Complete(d.Id, DelegationState.Failed, "busy")!; + + var card = DelegationRegistry.CompletionLine(failed); + + Assert.Contains("did not finish", card); + Assert.Contains("busy", card); + } + + [Fact] + public void AnEmptyResultStillProducesAReadableCard() + { + var reg = new DelegationRegistry(); + var d = reg.Open("a", "Falchion", "b", "Ninja", "build the site"); + var done = reg.Complete(d.Id, DelegationState.Done, "")!; + + var card = DelegationRegistry.CompletionLine(done); + + Assert.Contains("Ninja finished", card); + Assert.DoesNotContain("—", card); // no trailing dash with nothing after it + } + + // ---- The registry ----------------------------------------------------------- + + [Fact] + public void AnAgentSeesOnlyTheJobsItHandedOut() + { + var reg = new DelegationRegistry(); + reg.Open("a", "Sonic", "b", "Ninja", "one"); + reg.Open("c", "Falchion", "b", "Ninja", "two"); + + var mine = reg.OpenedBy("a"); + + Assert.Single(mine); + Assert.Equal("one", mine[0].Task); + } + + [Fact] + public void FinishedJobsAreSweptButRunningOnesSurvive() + { + var reg = new DelegationRegistry(); + var running = reg.Open("a", "Sonic", "b", "Ninja", "still going"); + var done = reg.Open("a", "Sonic", "b", "Ninja", "over"); + reg.Complete(done.Id, DelegationState.Done, "fine"); + + reg.Sweep(TimeSpan.Zero); // everything finished is old enough + + Assert.NotNull(reg.Get(running.Id)); + Assert.Null(reg.Get(done.Id)); + } +} diff --git a/src/MandoCode.Desktop.Tests/MandoCode.Desktop.Tests.csproj b/src/MandoCode.Desktop.Tests/MandoCode.Desktop.Tests.csproj index e097e44..b8bbccc 100644 --- a/src/MandoCode.Desktop.Tests/MandoCode.Desktop.Tests.csproj +++ b/src/MandoCode.Desktop.Tests/MandoCode.Desktop.Tests.csproj @@ -61,6 +61,13 @@ + + + + + + +