feat(round-tables): ephemeral multi-agent deliberation sessions

[dundas]

Summary

• Implements the Round Table protocol on top of the existing ADMP Groups API (spec from decisive.gm, 2026-02-28)
• A Round Table is a temporary, goal-oriented session where agents deliberate on a topic and a facilitator resolves with an outcome
• Backing ADMP group is auto-created for multicast routing; participants receive a work_order invitation on creation

Endpoints

Method	Path	Description
POST	/api/round-tables	Create session (facilitator = caller)
GET	/api/round-tables	List sessions (filter by status, participant)
GET	/api/round-tables/:id	Read session (participants only)
POST	/api/round-tables/:id/speak	Append to thread + multicast to group
POST	/api/round-tables/:id/resolve	Close session with outcome (facilitator only)

Schema

{
  "id": "rt_abc123",
  "topic": "Should CircleSync v2 use push or pull?",
  "goal": "Reach consensus on sync strategy",
  "facilitator": "decisive.gm",
  "participants": ["circlesync.gm", "bootup.gm"],
  "group_id": "group://round-table-rt_abc123",
  "status": "open | resolved | expired",
  "thread": [{ "id": "...", "from": "circlesync.gm", "message": "...", "timestamp": "..." }],
  "outcome": null,
  "created_at": "2026-02-28T18:00:00Z",
  "expires_at": "2026-02-28T18:30:00Z"
}

Safety Invariants (from adversarial review)

• Max 20 participants per session
• Max 200 thread entries per session
• Backing group deleted on resolve and expiry — no orphaned group records
• Expiry handled in existing cleanup loop (alongside message/lease expiry)
• All endpoints enforce participant/facilitator membership

Files Changed

• src/routes/round-tables.js — 5 REST endpoints
• src/services/round-table.service.js — business logic (create, speak, get, resolve, list, expireStale)
• src/storage/memory.js — roundTables Map + CRUD methods
• src/server.js — route registration + cleanup loop hook

Test plan

• Full lifecycle test: create → speak → resolve → verify 409 on late speak
• Non-participant cannot read or speak (403)
• Non-facilitator cannot resolve (403)
• Participants cap enforced at 20 (400)
• Missing required fields return 400
• 148 total tests — 133 pass, 15 fail (all pre-existing outbox/Mailgun, unrelated)

🤖 Generated with Claude Code

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[claude]

PR Review: feat(round-tables): ephemeral multi-agent deliberation sessions

Overall this is a solid feature addition. Clean structure, good test coverage for the happy path, and the safety invariants (participant cap, thread cap, facilitator-only resolve) are all properly enforced. A few issues below range from bugs to security concerns to style nits.

---

Bugs / Correctness

1. Memory leak: resolved and expired round tables are never purged

expireStale updates status to 'expired' but never removes records from the roundTables Map. Same for resolve. The backing group is deleted but the round table object lives in memory forever. A deleteRoundTable(id) storage method is needed, called after a cooldown window (or immediately if post-resolution reads are not needed).

2. Race condition in speak thread-length guard

In round-table.service.js:106-118, two concurrent callers can both read thread.length = 199, both pass the check, and both write a 200-entry thread, yielding a final thread of 201. In the current in-memory backend this is unlikely (synchronous Map ops), but any async or persistent storage adapter will expose this TOCTOU issue. Moving the length check into updateRoundTable or using an atomic push-and-count would fix it.

3. updateRoundTable silently returns null on missing ID

In storage/memory.js:594, if the record does not exist the method returns null. The service never checks this return value, so a missing-ID update is a silent no-op rather than a thrown error.

---

Security

4. GET /api/round-tables?participant=<other-agent> leaks membership data

In routes/round-tables.js:59-62, any authenticated agent can pass ?participant=victim-agent and enumerate all round tables that agent participates in. This reveals which deliberations a third party is involved in. The participant query param should be restricted to req.agent.agent_id.

5. Participant IDs are not validated against registered agents

Arbitrary strings can be stored in participants. The addMember failures are silently swallowed, so a round table can be created where none of the listed participants actually exist or received invitations. Consider validating participants against registered agents before creating the session, or at minimum returning a summary of failed additions in the response.

6. No length cap on topic or goal

Both fields are validated as non-empty but are unbounded in size. A large topic string would be embedded in every thread multicast body and every work_order. A reasonable cap (e.g. 500 chars for topic, 2000 for goal) should be added.

7. timeout_minutes type not validated in the route

timeout_minutes: timeout_minutes || 30 is forwarded without checking it is a number. Sending "abc" causes NaN < 1 to evaluate as false, silently bypassing the service-layer range check. The route should reject non-numeric values explicitly.

---

Code Quality

8. console.warn instead of structured logger

The codebase uses a structured logger instance (visible in server.js). The service uses console.warn throughout, so round table warnings are excluded from structured log output and cannot be filtered or sampled alongside other service logs.

9. Error-message string-matching for status codes is fragile

getErrorStatusCode in the routes file maps HTTP status codes by matching substrings of error messages. A phrasing change in the service silently breaks the mapping. Typed errors (subclasses or a stable error.code property) would decouple the two layers.

10. Duplicate participants not rejected

participants: ["a", "a", "a"] is accepted, causing three invitations to be sent to the same agent. An explicit dedup check or rejection would be cleaner.

11. decision defaults to 'approved' silently

Resolving without a decision field silently records 'approved'. If decision is semantically meaningful (approved/rejected/deferred), it should either be required or the default should be clearly documented in the API guide.

---

Test Coverage Gaps

• GET /api/round-tables list endpoint: not tested at all (no test for default participant filter, ?status= filter, or the cross-agent privacy issue in feat: Docker deployment, OpenAPI documentation, and comprehensive testing #4).
• Expiry flow: no test that expireStale transitions status to 'expired' and that a subsequent /speak returns 409.
• Message length boundary: the 10,000-char limit in the route is untested.
• timeout_minutes validation: no test for out-of-range or non-numeric values.

---

Minor / Nit

• artifact_id: null is stored on every round table but is not mentioned in the PR schema and is not used anywhere. Remove it or document its intent.
• If groupService.delete fails silently on resolve or expiry, orphaned groups can accumulate with no way to find them. A logged warning is fine for now but worth a follow-up.

---

Overall the implementation is clean and the core invariants hold. Main items I would want addressed before merge: #4 (participant list data exposure), #1 (memory leak), and #6/#7 (unbounded/unvalidated inputs). The race condition (#2) is low-risk today but worth fixing before adding a persistent storage adapter.

[greptile-apps]

Greptile Summary

This PR implements the Round Table protocol as an ephemeral, goal-oriented multi-agent deliberation system built on top of the existing ADMP Groups API. The implementation adds 5 REST endpoints for creating, reading, speaking in, listing, and resolving round table sessions.

Key changes:

• New Round Table service creates backing ADMP groups for multicast routing and sends work_order invitations to participants
• Thread-based deliberation with 200 entry cap per session and 20 participant cap enforced
• Automatic expiry handling integrated into existing cleanup loop with group deletion on resolve/expiry
• Comprehensive authorization: facilitator-only resolution, participant-only read/speak access
• Full test coverage including lifecycle, authorization, validation, and edge cases

Observations:

• Best-effort participant addition: if adding a participant to the backing group fails (e.g., agent doesn't exist yet), creation proceeds but that participant may miss group multicasts
• Group cleanup on resolve/expiry catches and logs deletion errors without failing the operation, accepting the trade-off between strict cleanup guarantees and operation reliability
• List endpoint defaults to filtering by requester but allows querying other agents' sessions (though content access remains protected)

Confidence Score: 4/5

• Safe to merge with minor operational considerations around group cleanup and participant synchronization
• Strong implementation with proper authentication, validation, and test coverage. The best-effort approach to participant group membership and cleanup is a pragmatic trade-off for distributed systems. Score is 4 (not 5) due to potential edge cases where backing groups may not perfectly sync with participant lists.
• No files require special attention - the service layer handles errors gracefully with appropriate logging

Important Files Changed

Filename	Overview
src/routes/round-tables.js	New REST API routes for Round Table CRUD operations with proper authentication, validation, and error handling
src/services/round-table.service.js	Core business logic for ephemeral deliberation sessions with ADMP group integration, participant notifications, and automatic cleanup on expiry
src/storage/memory.js	Added roundTables Map with standard CRUD operations and participant filtering support
src/server.js	Registered round-tables routes and integrated expiry check into existing cleanup loop
src/server.test.js	Added comprehensive test coverage for Round Table lifecycle, authorization, validation, and edge cases

Sequence Diagram

sequenceDiagram
    participant F as Facilitator
    participant API as Round Table API
    participant GS as Group Service
    participant IS as Inbox Service
    participant P1 as Participant 1
    participant P2 as Participant 2

    F->>API: POST /api/round-tables<br/>{topic, goal, participants}
    API->>GS: create(round-table-rt_xxx)
    GS-->>API: group_id
    loop For each participant
        API->>GS: addMember(group_id, participant)
        API->>IS: send work_order invitation
    end
    API-->>F: 201 {id, status: open, group_id}

    P1->>API: POST /api/round-tables/:id/speak<br/>{message}
    API->>API: Append to thread
    API->>GS: postMessage(group_id, entry)
    GS->>P1: Multicast to group members
    GS->>P2: Multicast to group members
    API-->>P1: 201 {thread_entry_id}

    P2->>API: GET /api/round-tables/:id
    API-->>P2: 200 {thread, status}

    F->>API: POST /api/round-tables/:id/resolve<br/>{outcome}
    API->>API: Update status: resolved
    API->>GS: postMessage(resolution)
    API->>GS: delete(group_id)
    API-->>F: 200 {status: resolved, outcome}

Loading

_{Last reviewed commit: f586c1c}

[claude]

Code Review — feat(round-tables): ephemeral multi-agent deliberation sessions

Overall this is a well-structured addition. The layering (route → service → storage) is consistent with the rest of the codebase, safety invariants are clearly documented, and the test coverage covers the main happy-path and access-control scenarios. A few issues are worth addressing before merge.

---

Bugs

1. Silent participant exclusion vs. rt.participants inconsistency — High

In round-table.service.js create():

for (const participantId of uniqueParticipants) {
  try {
    await groupService.addMember(group.id, facilitator, participantId, 'member');
  } catch (err) {
    logger.warn({ participantId, ... }, '[RoundTable] Could not add participant');
  }
}

groupService.addMember throws Agent ${agentId} not found when the agent is not registered. Because that error is swallowed, the round table record stores the agent in rt.participants, but the backing group does not include them as a member. This creates a split-brain: speak multicasts to the group (missing the agent), but get and _requireParticipant treat them as legitimate participants. Consider failing fast on unknown participants, or removing failed enrollees from rt.participants before persisting.

---

2. timeout_minutes || 30 treats 0 as missing — Medium

In round-tables.js line 56:

timeout_minutes: timeout_minutes || 30

If a caller explicitly passes timeout_minutes: 0, the falsy coercion silently resets it to 30, bypassing the service min-1 validation entirely. Use nullish coalescing instead:

timeout_minutes: timeout_minutes ?? 30

---

3. Non-integer timeout_minutes passes route validation — Low

Line 47 accepts any finite number including 1.5. The service silently accepts it too. Either document that fractional minutes are accepted or tighten the check:

if (timeout_minutes !== undefined && (!Number.isInteger(timeout_minutes) || timeout_minutes < 1)) { ... }

---

Security / Correctness

4. getErrorStatusCode is string-fragile — Medium

Lines 13-19 infer HTTP status codes by inspecting raw error message strings:

if (msg.includes('Not a participant') || msg.includes('Only the facilitator')) return 403;

Any future rewording of service error messages will silently change the HTTP response code with no test coverage to catch it. Given the security relevance of 403 vs. 400/500, prefer typed errors (e.g. a custom class ForbiddenError with statusCode = 403) or a code property on thrown errors.

---

5. groupService.delete does not verify caller authorization — Low

In group.service.js the delete method accepts an agentId argument but never checks it before deleting. The round-table service passes rt.facilitator (correct), but the group service provides no defense-in-depth. Low risk since group IDs are internal and not user-controllable, but worth noting for a future hardening pass.

---

Missing Functionality

6. No expiry notification to participants — Medium

expireStale() silently marks sessions as expired and deletes the backing group. Participants are never told the session has closed, which is asymmetric with resolve(), which multicasts a resolution event. Consider sending a notification on expiry, or document the omission as intentional.

---

7. Resolved/expired round tables accumulate in memory forever — Low

There is no deleteRoundTable storage method and the cleanup loop only changes status. In a long-running process every historical session remains in the roundTables Map. A TTL-based purge (e.g. 24 hours after resolved_at/expires_at) would keep memory bounded, consistent with how cleanupExpiredMessages works elsewhere.

---

8. No pagination on GET /api/round-tables — Low

listRoundTables returns the full unfiltered result set for the caller. A limit/offset or cursor parameter would be consistent with how the inbox list endpoints work.

---

Test Coverage Gaps

9. Missing: facilitator can speak

The tests verify participants can speak and outsiders cannot, but not that the facilitator (who is not in rt.participants) can also contribute. _requireParticipant grants facilitator access via the rt.facilitator === agentId branch — this should be an explicit test case.

10. Missing: duplicate participant deduplication

A test that passes ["agent-a", "agent-a"] and asserts participants.length === 1 in the response would lock in the deduplication behaviour.

11. Missing: expireStale lifecycle

There is no test that creates an already-expired round table and asserts expireStale() returns 1 and cleans up the backing group. The cleanup path is entirely untested.

12. Missing: goal field validation

The validation test exercises missing topic and empty participants, but not a missing goal.

---

Minor Nits

• speak returning 201 and resolve returning 200 is reasonable, but a brief comment in the handler would prevent future reviewers from flagging it as an oversight.
• decision defaults to 'approved' when omitted but no valid values are documented or validated. An explicit enum (approved | rejected | deferred) would make the schema self-documenting.
• The work_order invitation embeds a hard-coded server path in instructions. If the server is behind a reverse proxy this may not match the public URL. Consider a BASE_URL env var or omitting the absolute path.

---

Summary

Severity	Count
High	1
Medium	3
Low	4
Nit	3

The participant/group inconsistency (item 1) and the expiry notification gap (item 6) are the most important to resolve before merge. Everything else can land as follow-ups.

Generated with Claude Code