From 0d1ddcc4df17809bf48ef33cc544b2e4c93296b2 Mon Sep 17 00:00:00 2001 From: EugeneTes Date: Wed, 7 Oct 2026 14:29:50 +0000 Subject: [PATCH] fix(a2a): unambiguous session scopes and validated context ids --- docs/a2a.md | 6 +++-- .../specs/2026-10-07-a2a-server-design.md | 4 +++- .../Agent/A2ASessionPool.cs | 10 ++++++-- .../Hosting/VolcanicA2AServer.cs | 5 +++- .../ServerEndToEndTests.cs | 23 +++++++++++++++++++ .../SessionPoolTests.cs | 10 ++++++++ 6 files changed, 52 insertions(+), 6 deletions(-) diff --git a/docs/a2a.md b/docs/a2a.md index 356abab..9c56b2a 100644 --- a/docs/a2a.md +++ b/docs/a2a.md @@ -78,8 +78,10 @@ restart to reach that one endpoint (`/.well-known/agent-card.json` and the exten ## Sessions One `AgentSession` per `(caller, contextId)`, opened or resumed the first time a message names that `contextId` -(a missing one gets a fresh UUID). Scope: `-a2a--` (the Telegram host's -`ScopePrefix` is `telegram-`). The session runs in the caller's workspace, so it loads that workspace's +(a missing one gets a fresh UUID). A client-chosen `contextId` must match `[A-Za-z0-9_-]{1,100}`, otherwise the +message is refused with `InvalidParams`. Scope: `-a2a-_` (the Telegram host's +`ScopePrefix` is `telegram-`); caller names never contain `_`, so two different callers or contexts can't +share a session. The session runs in the caller's workspace, so it loads that workspace's `AGENTS.md` (or `CLAUDE.md`), `PERSONA.md` and `MEMORY.md` exactly like any other session there — same persona, same memory, separate conversation. It does **not** get a calendar (Google Calendar connections are keyed by session scope, and an A2A scope never connects one). diff --git a/docs/superpowers/specs/2026-10-07-a2a-server-design.md b/docs/superpowers/specs/2026-10-07-a2a-server-design.md index 485363d..8e1af0e 100644 --- a/docs/superpowers/specs/2026-10-07-a2a-server-design.md +++ b/docs/superpowers/specs/2026-10-07-a2a-server-design.md @@ -123,7 +123,9 @@ reads only `/.volcanic/a2a/tasks/` and also requires `task.cal 1. **Hops**: read `metadata["volcanic.hops"]` (missing = 0). Above `MaxHops` → the task is created and immediately `failed` with "hop limit reached". -2. **Context**: missing `contextId` → new UUID. Session scope `-a2a--`, cwd = caller +2. **Context**: missing `contextId` → new UUID; a given one not matching `[A-Za-z0-9_-]{1,100}` → `InvalidParams` + (checked before anything is created). Session scope `-a2a-_` (caller names are + `[a-z0-9-]`, so the first `_` separates name and context: no two pairs share a scope), cwd = caller workspace, `Channel` = a fresh `A2AChatChannel`, `ExtraExtensions` = `A2ASessionExtension`. `A2ASessionPool` opens or resumes it. 3. **Task**: diff --git a/src/Volcanic.AgenticCore.A2A.Server/Agent/A2ASessionPool.cs b/src/Volcanic.AgenticCore.A2A.Server/Agent/A2ASessionPool.cs index 8e0ca54..819f687 100644 --- a/src/Volcanic.AgenticCore.A2A.Server/Agent/A2ASessionPool.cs +++ b/src/Volcanic.AgenticCore.A2A.Server/Agent/A2ASessionPool.cs @@ -11,7 +11,8 @@ using Volcanic.AgenticCore.Hosting; namespace Volcanic.AgenticCore.A2A.Server.Agent; /// -/// One per caller context (scope <prefix>-a2a-<caller>-<contextId>) in the +/// One per caller context (scope <prefix>-a2a-<caller>_<contextId>, see +/// ) in the /// caller's workspace, so it loads that workspace's AGENTS.md/CLAUDE.md, PERSONA.md and MEMORY.md. One run at a time per /// context. Idle sessions are disposed after and resumed from JSONL later. /// @@ -68,7 +69,12 @@ internal sealed class A2ASessionPool : IA2ALiveRuns, IAsyncDisposable _evictTimer = time.CreateTimer(_ => _ = EvictIdleAsync(), null, TimeSpan.FromMinutes(1), TimeSpan.FromMinutes(1)); } - public static string ScopeFor(A2ACaller caller, string contextId) => $"{caller.ScopePrefix}-a2a-{caller.Name}-{contextId}"; + /// + /// <prefix>-a2a-<caller>_<contextId>. Caller names are [a-z0-9-] (no _), so the first + /// _ after the prefix separates the name from the client-chosen context id ([A-Za-z0-9_-]{1,100}, checked + /// by the server): two different (caller, context) pairs never share a scope. + /// + public static string ScopeFor(A2ACaller caller, string contextId) => $"{caller.ScopePrefix}-a2a-{caller.Name}_{contextId}"; public bool IsLive(string taskId) { diff --git a/src/Volcanic.AgenticCore.A2A.Server/Hosting/VolcanicA2AServer.cs b/src/Volcanic.AgenticCore.A2A.Server/Hosting/VolcanicA2AServer.cs index 11b05cb..e9b3e9d 100644 --- a/src/Volcanic.AgenticCore.A2A.Server/Hosting/VolcanicA2AServer.cs +++ b/src/Volcanic.AgenticCore.A2A.Server/Hosting/VolcanicA2AServer.cs @@ -9,7 +9,7 @@ using Volcanic.AgenticCore.A2A.Server.Tasks; namespace Volcanic.AgenticCore.A2A.Server.Hosting; /// -/// The SDK server for one caller. Adds the pre-checks (busy context, file parts, continuation of a task that isn't +/// The SDK server for one caller. Adds the pre-checks (context id format, busy context, file parts, continuation of a task that isn't /// waiting), push-config storage in the task file and the per-workspace extended card. /// internal sealed class VolcanicA2AServer( @@ -32,6 +32,9 @@ internal sealed class VolcanicA2AServer( private void PreCheck(SendMessageRequest request) { var message = request.Message ?? throw new A2AException("A message is required.", A2AErrorCode.InvalidParams); + // Context ids name the session scope (and its directory): only [A-Za-z0-9_-]{1,100}. + if (message.ContextId is { Length: > 0 } requested && !A2ATaskFiles.IsValidId(requested)) + throw new A2AException("contextId must match [A-Za-z0-9_-]{1,100}.", A2AErrorCode.InvalidParams); foreach (var part in message.Parts ?? []) { if (part.Raw is { } raw && raw.Length > options.Value.MaxFileBytes) diff --git a/tests/Volcanic.AgenticCore.A2A.Server.Tests/ServerEndToEndTests.cs b/tests/Volcanic.AgenticCore.A2A.Server.Tests/ServerEndToEndTests.cs index 10bd48d..ff8b829 100644 --- a/tests/Volcanic.AgenticCore.A2A.Server.Tests/ServerEndToEndTests.cs +++ b/tests/Volcanic.AgenticCore.A2A.Server.Tests/ServerEndToEndTests.cs @@ -70,6 +70,29 @@ public class ServerEndToEndTests Assert.Contains("first", transcript); } + [Theory] + [InlineData("../x")] + [InlineData("a b")] + [InlineData("ctx.1")] + public async Task Send_BadContextId_InvalidParams(string contextId) + { + await using var host = await A2ATestServer.StartAsync(); + var ex = await Assert.ThrowsAsync(() => + host.Client().SendMessageAsync(Requests.Text("hi", contextId: contextId), CancellationToken.None)); + Assert.Equal(A2AErrorCode.InvalidParams, ex.ErrorCode); + Assert.Empty(host.Faux.Requests); + } + + [Fact] + public async Task Send_TooLongContextId_InvalidParams() + { + await using var host = await A2ATestServer.StartAsync(); + var ex = await Assert.ThrowsAsync(() => + host.Client().SendMessageAsync(Requests.Text("hi", contextId: new string('a', 200)), CancellationToken.None)); + Assert.Equal(A2AErrorCode.InvalidParams, ex.ErrorCode); + Assert.Empty(host.Faux.Requests); + } + [Fact] public async Task Send_ProviderError_TaskFailed() { diff --git a/tests/Volcanic.AgenticCore.A2A.Server.Tests/SessionPoolTests.cs b/tests/Volcanic.AgenticCore.A2A.Server.Tests/SessionPoolTests.cs index bc6d042..f9e69df 100644 --- a/tests/Volcanic.AgenticCore.A2A.Server.Tests/SessionPoolTests.cs +++ b/tests/Volcanic.AgenticCore.A2A.Server.Tests/SessionPoolTests.cs @@ -102,6 +102,16 @@ public sealed class SessionPoolTests : IAsyncDisposable Assert.NotNull(run.FinishedAt); } + [Fact] + public void Scope_DashInCallerOrContext_NeverCollides() + { + var tester = _caller with { Name = "tester" }; + var testerX = _caller with { Name = "tester-x" }; + Assert.Equal("p-a2a-tester_x-ctx", A2ASessionPool.ScopeFor(tester, "x-ctx")); + Assert.NotEqual(A2ASessionPool.ScopeFor(tester, "x-ctx"), A2ASessionPool.ScopeFor(testerX, "ctx")); + Assert.NotEqual(A2ASessionPool.ScopeFor(tester, "x_ctx"), A2ASessionPool.ScopeFor(testerX, "ctx")); + } + [Fact] public async Task Pool_Acquire_SetsTrackedEntry() {