feat(groups): shared artifacts - typed co-edited documents with CAS and validators (I17)

docs/changelog.md

@@ -3,6 +3,60 @@
 > **Purpose:** Living document tracking all changes, decisions, and reasoning during implementation. Updated as work progresses for easy reference and review.
 
 
+---
+
+## 🔎 fix(groups): I17 PR #637 review round 2 (2026-08-08)
+
+**Repo:** EDDI (`feat/group-i17-shared-artifacts`)
+
+Two comments triaged:
+
+- **Announce mutex no longer held across listener callbacks (CodeRabbit Major, accepted):** `announceArtifactChanges` held `artifactAnnounceMutex` through `onArtifactUpdated`, so one slow/backpressured SSE client blocked every other turn's end-of-turn drain. Now the mutex guards only the HANDOFF: exactly one thread at a time is the publisher — it drains under the mutex, releases it, fires the callbacks, and loops for late arrivals; every other thread sees the publisher flag and leaves, its changes guaranteed to ride the publisher's next pass. Write order preserved (single announcer, FIFO queue), no caller ever blocks on a listener. New test drives a write + reentrant announce from INSIDE a callback: published exactly once, in order, nothing stranded, no deadlock.
+- **CodeQL log-injection (stale):** raised against the initial commit 6aeba1393; the flagged attach-artifacts log was sanitized in round 1 (74c0acaf7). Reply-only.
+
+`MemberTurnExecutorTest` (14) + artifact suites green.
+
+---
+
+## 🔎 fix(groups): I17 PR #637 review round 1 (2026-08-08)
+
+**Repo:** EDDI (`feat/group-i17-shared-artifacts`)
+
+All 11 review comments (CodeRabbit ×9, Copilot/CodeQL ×2) triaged; every one accepted and fixed:
+
+- **Meta-schema validation at save time** (`ArtifactValidators.schemaSpecProblem`): `getSchema(spec)` only parses — `{"type":"strng"}` passed and misbehaved at write time. Specs now also validate *as instances* against the bundled 2020-12 meta-schema (no network I/O; degrades to parse-only with a WARN if the bundled resource can't load, rather than rejecting every config).
+- **ReDoS bound on REGEX validators**: config-authored pattern × 256 KB LLM content could backtrack catastrophically and pin the member turn. `checkRegex` now matches through a deadline-guarded `CharSequence` (500 ms, sampled every 1024 char accesses) and refuses the write on expiry — fails closed, like every other broken-spec path.
+- **`[null]` validator entries**: `List.copyOf` NPE'd during config deserialization, preempting `requireValidSpecs`' positional message; now an unmodifiable null-tolerant copy.
+- **Artifact event ordering + late writes**: drain+announce now holds a per-conversation mutex (two PARALLEL turns ending together could split the queue and publish v2 before v1), and `executeDiscussion`'s `finally` runs one **final announce pass per leg** so a write accepted by a timed-out member's still-running agent is announced instead of stranded. A write after even that pass keeps the artifact — only its live event is best-effort, by design.
+- **`listByGroupConversationId` order**: both backends sort DESC; the interface promises oldest-first. Now re-sorted in Java per the contract.
+- **`deleteByGroupConversationId`**: same processed-set/no-progress guard as `deleteAllForUser` — an undislodgeable row is counted once and ends the loop instead of spinning `MAX_ERASURE_PASSES` times and inflating the count.
+- **Slack mrkdwn injection**: artifact name/editor id are LLM-authored; `<!channel>` in a name rendered as a real broadcast. Both fields now `&`/`<`/`>`-escaped.
+- **Oversize refusal rounds up** (`Math.ceilDiv`): MAX+1 bytes no longer reads "256 KB is over the 256 KB limit".
+- **GDPR cascade Javadoc** now names the shared-artifact step; **CodeQL log injection** at `populateArtifacts` sanitized.
+
+**Tests:** +7 (meta-schema reject, null-entry positional message, catastrophic-regex deadline, late-write announce pass, single-pass write order, oldest-first sort, no-spin cascade delete). Touched suites 1961 tests — green except the 27 known environmental socket-bound errors (SafeHttpClient/SlackWebApi/Weather/WebScraper), which fail identically on an untouched tree.
+
+---
+
+## 📄 feat(groups): I17 — shared artifacts (blackboard-lite) (2026-08-08)
+
+**Repo:** EDDI (`feat/group-i17-shared-artifacts`)
+
+First Wave 2 queue item from `planning/group-collaboration-NEXT.md` §3. Agents can now **co-edit typed documents** instead of only talking: four member tools — `createArtifact`, `readArtifact`, `proposeArtifactUpdate`, `listArtifacts` — gated by a new `artifactConfig` on the group config.
+
+**Design decisions, per the plan (and the plan's own rejections honored):**
+
+- **Own collection, never embedded.** `SharedArtifact` + `ISharedArtifactStore`/`SharedArtifactStore` follow `GroupConversationStore`'s single-version runtime-document pattern. The discussion loop's whole-document stale-snapshot persists cannot clobber artifact writes, which is also why — unlike I5's task tools — the artifact tools write **through the store directly**. The live registry is still consulted: membership at assembly (`getForMember`, the caller-supplied-id IDOR guard), liveness at write time, and accepted writes ride a new transient change queue on the live `GroupConversation`.
+- **Deterministic CAS-and-retry, explicitly not an LLM fusion arbiter.** The version CAS needed a storage primitive that doesn't exist for numbers: `storeIfFieldEquals(String)` text-compares, which "works" on Postgres (`data->>` renders JSON numbers as text) and **silently never matches on Mongo** (typed BSON equality). New `storeIfFieldEquals(…, long)` overload on `IResourceStorage` + both backends, same no-silent-degrade contract (the default throws). Stale writers get the plan's sentence: *"artifact changed since you read it (now vN); re-read and merge your change."*
+- **Declarative validators only.** `JSON_SCHEMA` (new dependency `com.networknt:json-schema-validator` — the victools libraries only *generate* schemas), `REGEX`, `MAX_LENGTH`. Specs hard-fail the config save (`ArtifactValidators.requireValidSpecs` from `AgentGroupStore`, `HitlConfigValidation`'s contract); write-time failures are rejection sentences and the gate fails closed on a broken spec. Content ≤ 256 KB.
+- **Events without a listener reference:** tools can't fire SSE/Slack events (`ToolAssemblyContext` carries no listener — the structural gap that left I5's planned `task_added_by_agent` unfired). Accepted writes queue an `ArtifactChange` on the live instance; `MemberTurnExecutor` drains the queue in a `finally` after every turn and fires the new `artifact_updated` event (sink constant + record + SSE forward + Slack line + OpenAPI description lists). Drained even with a null listener so the queue cannot grow unbounded.
+- **Lifecycle:** artifacts are attached to the discussion status payload at read time in the service (so REST *and* MCP `read_group_conversation` carry them — `availableActions` idiom, `READ_ONLY`, never trusted back from storage); close/delete cascade removes them (`GroupLifecycleOps`, warn-and-continue so a broken artifact store can't make discussions undeletable); GDPR erasure sweeps them **user-keyed** via a stamped `ownerUserId` (page/exact-recheck/fail-loud contract copied from the group store) as a new `GdprComplianceService` cascade step.
+- **Caps:** `maxArtifactsPerDiscussion` (default 5) counted inside a `synchronized (liveInstance)` block — creation is check-then-act and PARALLEL phases genuinely race; updates need no lock, the CAS decides.
+
+**Tests (148 across 8 classes, all green):** tools against a real in-memory CAS store (stale-version retry sentence with the CURRENT version, concurrent same-version writers → exactly one winner, FINAL freeze, foreign-discussion ids don't resolve, validator chain, refusals leave no side effect); provider gate matrix (every uncertainty → contribute nothing, membership not existence, `enableBuiltInTools` still applies); store CAS through the numeric overload with `verify(never()).store(…)`; anchored+escaped filters with Java exact-recheck; erasure paging/fail-loud; lifecycle cascade ordering (`inOrder` artifact-delete before document-delete) + cascade-failure-still-deletes; GDPR step + not-resolvable skip + failure-continues; turn-executor drain (exactly once, null-listener drain); Slack lines incl. degenerate-payload skip. **Mutation notes:** degrading the store CAS to an unconditional store does not even compile (the gone-document catch becomes unreachable) — the CAS call is structurally load-bearing; the Mockito-verified negatives (`never().store`, `specs().isEmpty()`) pin the rest.
+
+**Files:** `SharedArtifact`, `ISharedArtifactStore`, `SharedArtifactStore`, `ArtifactValidators`, `ArtifactTools`, `ArtifactToolsProvider` (+ `AgentOrchestrator` phase-1 wiring), `AgentGroupConfiguration` (`ArtifactConfig`/`ArtifactValidator`/`ValidatorKind`), `GroupConversation` (change queue + read-time `artifacts`), `IResourceStorage` + Mongo/Postgres (numeric CAS), `GroupConversationEventSink`/listener/SSE/Slack, `GroupLifecycleOps`, `GroupConversationService`, `GdprComplianceService`, `AgentGroupStore`, `pom.xml`, `docs/group-conversations.md`, 8 test classes.
+
 ---
 
 ## 🔀 merge: bring `origin/main` (PR #627 HITL request pinning) into the branch (2026-08-07)

docs/group-conversations.md

@@ -160,6 +160,46 @@ Both caps are enforced independently: `maxPerTurn` bounds a runaway single turn,
 discussion cap counts only agent-filed tasks, so a large planned backlog does not
 exhaust it. A rejected call does not consume the per-turn budget.
 
+## Shared artifacts (blackboard-lite)
+
+Without artifacts, the transcript is the only medium — every structured thing an
+agent produces is prose the next agent re-parses. `artifactConfig` gives members
+four tools to **create together**: `createArtifact(name, type, content)`,
+`readArtifact(nameOrId)`, `proposeArtifactUpdate(nameOrId, content,
+expectedVersion, markFinal?)` and `listArtifacts()`. Artifacts are typed
+documents (`TEXT`, `MARKDOWN`, `JSON`) in their own collection, listed on the
+discussion's REST/MCP status payload as `artifacts`, and announced over SSE and
+Slack as `artifact_updated` events.
+
+```json
+"artifactConfig": {
+  "allowArtifactTools": true,
+  "maxArtifactsPerDiscussion": 5,
+  "validators": [
+    { "kind": "JSON_SCHEMA", "spec": "{\"type\":\"object\",\"required\":[\"title\"]}" },
+    { "kind": "MAX_LENGTH", "spec": "20000" }
+  ]
+}
+```
+
+**Concurrency is deterministic compare-and-set, not an LLM merge.** Every update
+presents the version it read; a stale writer is told *"artifact changed since
+you read it (now v3); re-read and merge your change"* and retries against fresh
+content. The failure mode is a retry, never a silent bad merge.
+
+**Validators are declarative only** — `JSON_SCHEMA`, `REGEX` (content must
+contain a match), `MAX_LENGTH` (characters) — never code. Specs are checked at
+config save time; at write time a failing validator refuses the write with its
+message and stores nothing. Content is additionally capped at 256 KB per
+artifact.
+
+Off by default with the same absence discipline as the task tools: no opt-in
+means the tools are never assembled. The member agent's own
+`enableBuiltInTools` switch still applies. `markFinal: true` freezes an
+artifact — FINAL artifacts accept no further updates. Artifacts are deleted
+with their discussion (close/delete cascade) and by GDPR erasure; the durable
+trace of the work is the transcript.
+
 ## 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.

pom.xml

@@ -333,6 +333,13 @@
             <artifactId>jsonschema-module-jackson</artifactId>
             <version>4.38.0</version>
         </dependency>
+        <!-- JSON Schema VALIDATION (victools above only GENERATES schemas) — used
+             by the declarative shared-artifact validators (I17). -->
+        <dependency>
+            <groupId>com.networknt</groupId>
+            <artifactId>json-schema-validator</artifactId>
+            <version>1.5.4</version>
+        </dependency>
         <dependency>
             <groupId>jakarta.annotation</groupId>
             <artifactId>jakarta.annotation-api</artifactId>