Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions docs/attachments-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,14 @@ This can be useful for logging, debugging, or constructing custom prompts that r

---

## Group Conversations

Group discussions accept the same three input shapes on `POST /groups/{groupId}/conversations`. Inline `base64Data` is stored in the blob store owned by the group conversation; hosted `url` references and pre-uploaded `storageRef`s pass through.

Each member's private conversation is granted access on its **first** turn and receives the files as `attachment_*` context; from there everything on this page applies unchanged. Later turns rely on the member's own history plus the auto-enabled `readAttachment` tool, so a member does not lose the file after phase one.

Two group-specific bounds: the per-turn cap (`eddi.attachments.max-per-turn`) applies per **member turn**, and anything dropped is reported in that member's `attachments:errors`, not in the group transcript. See [group-conversations.md → Attachments](group-conversations.md#attachments).

## Architecture Notes

- **No inline storage**: Attachment payloads are never stored inline in conversation memory documents. Only metadata references are persisted.
Expand Down
18 changes: 18 additions & 0 deletions docs/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,24 @@
> **Purpose:** Living document tracking all changes, decisions, and reasoning during implementation. Updated as work progresses for easy reference and review.


---

## 📚 docs(groups): attachments, protocol defaults, context scopes, and a "not yet supported" section (2026-08-09)

**Repo:** EDDI (`docs/group-agent-accuracy`)

Wave E of the Agent / Group Agent review — the documentation drifts the review turned up, each one a place where the docs and the code disagreed.

1. **Attachments × groups were entirely undocumented.** `docs/group-conversations.md` had zero mentions of attachments and `docs/attachments-guide.md` had zero mentions of groups, while `POST /groups/{groupId}/conversations` accepts them and `GroupAttachmentBinder` is a whole subsystem. Both files now carry a section: the three input shapes, the first-turn grant, how later phases keep access (history + the auto-enabled `readAttachment` tool), and the two group-specific bounds — the per-turn cap applies per MEMBER turn, and anything dropped is reported in that member's `attachments:errors`, not in the group transcript.
2. **The protocol table did not say the defaults apply to an absent block.** Nothing backfills a stored config, so "no `protocol` block" is the common shape and runs on exactly the tabled values — which is what made the 60-vs-180 drift fixed in #648 invisible.
3. **`maxCreatedAgentsPerDiscussion` read as ambiguous.** Now explicitly "counted across **all** members, not per member" — the behaviour #649 delivers.
4. **`LAST_PHASE` was documented as "only the previous phase's entries"**, but the filter is `phaseIndex >= currentPhaseIdx - 1`, which includes the running phase. The code is right — in a sequential phase, that inclusion is what lets the second speaker react to the first — so the doc and the enum's Javadoc were corrected, not the filter.
5. **New "Not yet supported" section**, so these are not discovered at runtime: member-level tool approval inside a group, nested pauses, groups over the OpenAI-compatible `/v1` adapter, groups over A2A, and the per-node scope of the live-discussion registry.

Also: an FQN sweep of `CreateSubAgentTool` (11 inline fully-qualified names, against `AGENTS.md` §4.7) and the orphaned Javadoc block in `LiveDiscussionRegistry`, where the paragraph documenting `get()` sat above `getForMember()` so both attached to the latter and `get()` had none.

**Scoping note:** the FQN violation is repo-wide (~130 sites). This PR sweeps only files no other open PR touches; doing all of them here would collide with #648–#651 for no benefit. The rest is a follow-up once those land.

---

## 🔎 fix(groups): pre-merge deep review — facilitator HITL bypass, CALL_VOTE guards, metric cardinality, template honesty (2026-08-08)
Expand Down
42 changes: 40 additions & 2 deletions docs/group-conversations.md
Original file line number Diff line number Diff line change
Expand Up @@ -536,6 +536,30 @@ roles fail loudly, naming the template's real roles.
| `decision-board` | CUSTOM | Hybrid board with a HUMAN director (I6) deliberating and voting (I14); options distilled by the chair, ties to the chair |
| `negotiation-table` | NEGOTIATION | Typed two-party bargaining with concession ledger (I11); the arbiter role may be a human principal |

## Attachments

A discussion can carry shared files. `POST /groups/{groupId}/conversations` (and the `/stream` variant) accepts an `attachments` array alongside `question`, in the same three shapes the single-agent API takes:

```json
{
"question": "Review the attached architecture proposal.",
"attachments": [
{ "storageRef": "att_01J..." },
{ "mimeType": "application/pdf", "url": "https://example.com/proposal.pdf" },
{ "mimeType": "image/png", "fileName": "diagram.png", "base64Data": "iVBOR..." }
]
}
```

Inline `base64Data` is stored in the blob store owned by the group conversation, so it is granted to members and reaped with the conversation. Hosted `url` references and pre-uploaded `storageRef`s pass through as-is.

**How members receive them.** On a member's **first** turn the orchestrator grants that member's private conversation access to the group's blobs and injects them as `attachment_*` context — from there the ordinary single-agent attachment path applies (multimodal forwarding for vision models, PDF/text extraction otherwise). On **later** turns the member's own conversation history carries them: `AttachmentForwarder` notes the earlier attachments and the `readAttachment` tool is auto-enabled for any conversation that has them, independently of `builtInToolsWhitelist`. A recruited member gets the same grant on its own first turn, and a nested `GROUP` member propagates the whole set down.

**Bounds worth knowing:**

- The per-turn forwarding cap (`eddi.attachments.max-per-turn`, default 5) applies to each member turn. A discussion sharing more than that will have the surplus dropped for that turn, reported in the member's `attachments:errors` — visible in that member's conversation, **not** in the group transcript.
- `url`-only attachments are not blob-backed and are not re-hydrated after a HITL resume; blob-backed ones are.

## Nested Groups (Group-of-Groups)

Members can be other groups. The sub-group runs its own discussion and its synthesized answer becomes the member's response.
Expand Down Expand Up @@ -612,7 +636,7 @@ For full control, define phases directly:
|---|---|
| `NONE` | Only the question (independent) |
| `FULL` | All previous transcript entries |
| `LAST_PHASE` | Only the previous phase's entries |
| `LAST_PHASE` | The previous phase's entries **and the current phase's so far** — in a sequential phase, that is what lets the second speaker react to the first |
| `ANONYMOUS` | Previous entries with speaker names removed |
| `OWN_FEEDBACK` | Only feedback addressed to this agent |
| `TASK_ONLY` | Only this agent's assigned task from the plan |
Expand Down Expand Up @@ -745,7 +769,7 @@ Guardrails for dynamic agent creation are configured per-group via `AgentGroupCo
| `allowCreation` | `false` | Allow creating new agents (vs. only recruiting existing) |
| `allowRecruitment` | `false` | Allow recruiting already-deployed agents into the discussion |
| `allowDelegation` | `true` | Allow delegating sub-tasks to other agents |
| `maxCreatedAgentsPerDiscussion` | `5` | Cap on new agents created per discussion |
| `maxCreatedAgentsPerDiscussion` | `5` | Cap on new agents created per discussion — counted across **all** members, not per member |
| `maxRecruitedAgentsPerDiscussion` | `10` | Cap on recruited agents per discussion |
| `delegationTimeoutSeconds` | `60` | How long a delegating agent waits for its delegate's turn. Non-positive falls back to the default |
| `maxDelegationsPerTask` | `3` | Cap on delegations per task |
Expand Down Expand Up @@ -780,6 +804,8 @@ If tenant quotas are enabled, `QuotaExceededException` is propagated regardless
| `maxRetries` | 0+ | 2 |
| `onMemberUnavailable` | `SKIP`, `FAIL` | `SKIP` |

> The defaults above apply when the `protocol` block is **omitted entirely**, not only when an individual setting is. Nothing backfills a stored config, so a group saved without a `protocol` block runs on exactly these values.

> **Timeout guidance**: 180s covers thinking models (e.g. `claude-sonnet-5`) and synthesis phases comfortably. For tool-calling agents with multiple tool loops, consider `300`–`600`. The timeout is per agent turn, not per phase.

## REST API
Expand Down Expand Up @@ -853,6 +879,18 @@ Configure trigger keywords in `ChannelIntegrationConfiguration` to route to spec

After a discussion, users can reply in any agent's thread to ask follow-up questions. The system injects the agent's discussion context (contribution + peer feedback received) into the prompt for a contextual response.

## Not yet supported

Known bounds, so they are not discovered at runtime:

| Area | Behaviour today |
|---|---|
| **Member-level tool approval** | A member agent's `hitlConfig.toolApprovals` does not gate inside a group. The framework auto-rejects the gated call (`system:group`) and the member produces a tool-less contribution — see [hitl.md → Group members](hitl.md#group-members). `inGroupTurns: "INBOX"` is reserved and rejected at save time. |
| **Nested pauses** | A sub-group that pauses for approval or a human turn is cancelled and its member turn recorded `SKIPPED`. Nested HITL is not supported. |
| **Groups over the OpenAI-compatible API** | `/v1` exposes deployed *agents* as models. A group is not addressable there; use the REST or MCP surfaces. |
| **Groups over A2A** | An A2A peer can invoke an agent, not a group. |
| **Cross-node tool availability** | The task, artifact and recruit tools resolve the running discussion from an in-process registry. Member turns always run in the same JVM as their orchestrator, so this holds — but a discussion is only "live" on the node running it. |

## Configuration

```properties
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -894,7 +894,14 @@ public enum ContextScope {
NONE,
/** Agent sees entire transcript so far. */
FULL,
/** Agent sees only the immediately preceding phase. */
/**
* Agent sees the immediately preceding phase and the current one.
* <p>
* The current phase is included deliberately, and the doc used to omit it: in a
* sequential phase the second speaker's whole purpose is to react to the first,
* and excluding the running phase would leave every speaker after the first
* talking past their peers.
*/
LAST_PHASE,
/** Agent sees content from prior phases but not who said it. */
ANONYMOUS,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,13 +84,6 @@ public void unregister(String groupConversationId) {
live.remove(groupConversationId);
}

/**
* The live instance for a running discussion, or empty if it is not currently
* running (paused, finished, or never started on this node — group control is
* per-node, like {@code activeTokens}). Callers resolve this via the
* {@code groupConversationId} context var and must turn an empty result into an
* actionable error string for the LLM, never an exception.
*/
/**
* The live discussion ONLY if {@code conversationId} is one of its member
* conversations.
Expand Down Expand Up @@ -123,6 +116,16 @@ public Optional<GroupConversation> getForMember(String groupConversationId, Stri
&& gc.getMemberConversationIds().containsValue(conversationId));
}

/**
* The live instance for a running discussion, or empty if it is not currently
* running (paused, finished, or never started on this node — group control is
* per-node, like {@code activeTokens}). Callers must turn an empty result into
* an actionable error string for the LLM, never an exception.
* <p>
* <b>Never use this to authorize a tool</b> — see
* {@link #getForMember(String, String)}, which explains why existence is not
* membership.
*/
public Optional<GroupConversation> get(String groupConversationId) {
return Optional.ofNullable(live.get(groupConversationId));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@
import java.util.Set;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.ConcurrentHashMap;
import java.util.Objects;
import ai.labs.eddi.engine.memory.model.SimpleConversationMemorySnapshot;
import ai.labs.eddi.engine.memory.model.ConversationState;

/**
* LLM tool for dynamically creating sub-agents during group conversations.
Expand Down Expand Up @@ -62,8 +68,8 @@ public CreateSubAgentTool(AgentSetupService agentSetupService,
this.parentAgentId = parentAgentId;
this.userId = userId;
this.config = config != null ? config : new DynamicAgentConfig();
this.createdAgentIds = createdAgentIds != null ? createdAgentIds : new java.util.concurrent.CopyOnWriteArrayList<>();
this.retainedAgentIds = retainedAgentIds != null ? retainedAgentIds : java.util.concurrent.ConcurrentHashMap.newKeySet();
this.createdAgentIds = createdAgentIds != null ? createdAgentIds : new CopyOnWriteArrayList<>();
this.retainedAgentIds = retainedAgentIds != null ? retainedAgentIds : ConcurrentHashMap.newKeySet();
}

@Tool("Create a new sub-agent dynamically. The agent is set up, deployed, and optionally sent an initial message. "
Expand Down Expand Up @@ -102,7 +108,7 @@ public String createSubAgent(
&& config.getAllowedProviders() != null
&& !config.getAllowedProviders().isEmpty()) {
boolean providerAllowed = config.getAllowedProviders().stream()
.filter(java.util.Objects::nonNull)
.filter(Objects::nonNull)
.anyMatch(p -> p.equalsIgnoreCase(provider));
if (!providerAllowed) {
return "⚠️ Provider '%s' is not allowed. Allowed: %s"
Expand All @@ -120,7 +126,7 @@ public String createSubAgent(
List<String> allowedModels = config.getAllowedModels().entrySet().stream()
.filter(e -> e.getKey() != null && e.getKey().equalsIgnoreCase(provider))
.map(Map.Entry::getValue)
.filter(java.util.Objects::nonNull)
.filter(Objects::nonNull)
.findFirst().orElse(null);
if (allowedModels != null && !allowedModels.isEmpty()
&& allowedModels.stream().noneMatch(m -> m.equalsIgnoreCase(model))) {
Expand All @@ -131,9 +137,9 @@ public String createSubAgent(
// No provider specified — model must appear in at least one provider's
// allow-list
boolean modelFoundInAnyProvider = config.getAllowedModels().values().stream()
.filter(java.util.Objects::nonNull)
.filter(Objects::nonNull)
.flatMap(List::stream)
.filter(java.util.Objects::nonNull)
.filter(Objects::nonNull)
.anyMatch(m -> m.equalsIgnoreCase(model));
if (!modelFoundInAnyProvider) {
return "⚠️ Model '%s' is not in any provider's allowed models list."
Expand Down Expand Up @@ -186,8 +192,8 @@ public String createSubAgent(
InputData inputData = new InputData();
inputData.setInput(initialMessage);

CompletableFuture<ai.labs.eddi.engine.memory.model.SimpleConversationMemorySnapshot> responseFuture = new CompletableFuture<>();
final java.util.concurrent.atomic.AtomicBoolean skipped = new java.util.concurrent.atomic.AtomicBoolean();
CompletableFuture<SimpleConversationMemorySnapshot> responseFuture = new CompletableFuture<>();
final AtomicBoolean skipped = new AtomicBoolean();

// Finding H7: a busy-skip (onSkipped, e.g. IN_PROGRESS) must NOT be
// treated as a fresh response — the default onSkipped→onComplete would
Expand All @@ -197,12 +203,12 @@ public String createSubAgent(
false, true, null, inputData, false,
new IConversationService.ConversationResponseHandler() {
@Override
public void onComplete(ai.labs.eddi.engine.memory.model.SimpleConversationMemorySnapshot snapshot) {
public void onComplete(SimpleConversationMemorySnapshot snapshot) {
responseFuture.complete(snapshot);
}

@Override
public void onSkipped(ai.labs.eddi.engine.memory.model.SimpleConversationMemorySnapshot snapshot) {
public void onSkipped(SimpleConversationMemorySnapshot snapshot) {
skipped.set(true);
responseFuture.complete(snapshot);
}
Expand All @@ -211,7 +217,7 @@ public void onSkipped(ai.labs.eddi.engine.memory.model.SimpleConversationMemoryS
var snapshot = responseFuture.get(60, TimeUnit.SECONDS);
// Finding 25: a sub-agent that pauses for approval on its first
// message must report the pending approval, not "[no response]".
if (snapshot != null && snapshot.getConversationState() == ai.labs.eddi.engine.memory.model.ConversationState.AWAITING_HUMAN) {
if (snapshot != null && snapshot.getConversationState() == ConversationState.AWAITING_HUMAN) {
response = "PAUSED_FOR_APPROVAL: the sub-agent's conversation " + conversationId
+ " requires human approval before it can continue. A reviewer must decide via "
+ "POST /agents/" + conversationId + "/resume." + toolPauseSuffix(snapshot);
Expand All @@ -220,8 +226,8 @@ public void onSkipped(ai.labs.eddi.engine.memory.model.SimpleConversationMemoryS
// processed (busy or no longer active) — report accurately
// instead of returning the stale previous-turn output.
var state = snapshot != null ? snapshot.getConversationState() : null;
if (state == ai.labs.eddi.engine.memory.model.ConversationState.ENDED
|| state == ai.labs.eddi.engine.memory.model.ConversationState.EXECUTION_INTERRUPTED) {
if (state == ConversationState.ENDED
|| state == ConversationState.EXECUTION_INTERRUPTED) {
response = "[Sub-agent conversation " + conversationId + " is no longer active (state: "
+ state + "); initial message not delivered]";
} else {
Expand Down Expand Up @@ -296,7 +302,7 @@ private String toolPauseSuffix(ai.labs.eddi.engine.memory.model.SimpleConversati
if (batch != null && batch.getCalls() != null) {
var toolNames = batch.getCalls().stream()
.map(ai.labs.eddi.engine.memory.model.PendingToolCallBatch.PendingToolCall::getToolName)
.filter(java.util.Objects::nonNull)
.filter(Objects::nonNull)
.toList();
if (!toolNames.isEmpty()) {
suffix.append(" Gated tools: ").append(String.join(", ", toolNames)).append('.');
Expand Down
Loading