chore(monorepo): move EDDI-Manager and EDDI-Chat-UI into ui/ and build them with Maven - #757
Merged
Merged
Conversation
Nine inline comments on this PR. All valid; several were Major bugs in code this
PR introduced.
- maskIndentedCode corrupted real code. INDENTED_RUN stops at a blank line, so a
code block containing one arrives as several runs; for every run after the
first the nearest preceding non-blank line is the previous run's indented
code, which INDENTED_CONT matched, so the run was misread as list-continuation
prose and the rules rewrote it — " bar(c,D);" became " bar(c, D);". The
exact failure this function exists to prevent. Now tracks which lines were
already classified as code, with three tests pinning multi-run blocks.
- variables.tsx never got the "no data" guard its four sibling pages got, so a
failed window-focus refetch rendered the destructive ErrorState *in addition
to* the still-populated search bar and table.
- quotas.tsx had the same hole on its main quotaError branch: TanStack Query
keeps the last data, so a failed background poll replaced a populated policy
form with the full-page error. Both now fall back to RefetchErrorNotice.
- command-palette did not guard the parseResourceUri fallback. With a pathless
resource URI it returns the whole string as `id`, producing
/manage/agentview/eddi://… — the dead route this PR was fixing. Our own test
documented that shape. Falls back to the agent list, and encodes the id.
- parseSseFrame used .trim() on the event: value, contradicting the
optional-space-only rule documented directly above it, and turned an explicit
empty `event:` into type "" rather than the caller's default.
- channel-detail hand-rolled a destructive/red refetch banner while the four
sibling pages use the shared amber RefetchErrorNotice, misrepresenting a
still-editable state as a severe error.
- The audit gate matched the allowlist on GHSA id alone, so an accepted advisory
would have been suppressed if it later surfaced under a different production
package, where the "unreachable code path" argument may not hold. Now requires
the package to match; verified it fails on a mismatch.
- HANDOFF.md carried two different test totals; the stale block said 4127/281.
Replaced with a pointer to `npm test` rather than a third number to rot.
Not adopted: rescoping the stripComments regression guard in route-integrity.
Re-read the cited lines — that assertion is about stripComments not eating targets
on lines containing "//", which does not conflict with parseRouteTree stripping
comments first; they operate on different inputs.
…used reviews
Three follow-ups from the review backlog.
CI — the E2E workflow has been red on every push to main for weeks and the badge
meant nothing. The `backend-tests` job asserted `labsai/eddi:latest` was ALREADY
on the runner, which is never true on a fresh one, so both matrix legs failed
before running a test. `labsai/eddi` is published publicly (99 tags, `latest`
current), so pull it instead of asserting it. A `workflow_dispatch` input can pin
a specific build (e.g. 6.2.0-b764) and is aliased to `:latest` for the compose
files. Note this makes the job do real work for the first time, so it may surface
genuine backend failures that were previously masked by the early exit.
Workforce ?template= deep links did nothing. Three places link in with a template
pre-chosen — the onboarding hero's template cards and its Custom card, and
Dashboard > Templates > Use — and the wizard read no search params at all, so
every one landed on step 0 with nothing selected and Next disabled. Two id spaces
arrive here: built-ins keyed by `key`, saved templates by a crypto.randomUUID()
`id` in localStorage; both are resolved, and an unknown value leaves the picker
untouched rather than half-applying.
Human oversight was a no-op that also polluted config. What makes a discussion
pause is `phase.requiresApproval` (lib/hitl-config.ts calls it the sole trigger),
and the Workforce settings page never touches `phases` — so the section saved
successfully while nothing could ever pause. Worse, it wrote `hitlConfig`
unconditionally, inventing an approval policy on groups that had none, which the
Manager's editor then reads as "HITL enabled" with no phase gated. It now persists
hitlConfig only when the group already had one, and says plainly that approval
points are chosen in the Manager, with a link. Choosing phases here would be a new
feature, not a fix, so it is deliberately not added.
A paused discussion was unrecoverable from the board: a greyed-out composer read
"Awaiting approval…" with no approve/reject control and no route to one. Adds a
banner linking to the Manager group page where the approval actually lives.
Also fixes the pre-existing duplicate-React-key warning this branch kept seeing.
`getAgentDescriptorsWithVersions` runs one descriptor query per version and
flattens them, and the backend's `filter=` is a TEXT match — so the same version
arrived more than once (rendered as key={v.version}, React logged "two children
with the same key" and may drop or duplicate options), and an agent whose id
merely CONTAINS this one was offered in its version picker, where selecting a
version that does not exist here navigated nowhere useful. Resolved by keying on
the parsed id and version: this agent only, one entry per version. All four
consumers of useAgentVersions inherited both bugs.
4256 -> 4262 tests. Each fix verified to fail without it.
- A saved template applied via ?template=<uuid> forced selectedTemplate to
"custom", and that path hardcodes style CUSTOM / maxRounds 1 — so "Use
template" silently dropped the style and round count that the template stores
and the Templates panel displays, recreating only the member list. Both are
carried through now, and resolvedStyle reflects them on the review step.
- The hitlConfig change dropped user edits silently. This page cannot create a
hitlConfig (approval points, which are what actually gate a pause, live in the
Manager), so edits on a group without one were discarded while isDirty still
tracked them — leaving the page permanently dirty after a successful save. The
four controls are now disabled when there is nothing to persist to, and dirty
tracking ignores them in that case.
Also fixes a lint warning the first of those introduced: handleCreate read
savedTemplateConfig without listing it as a dependency — the same stale-closure
class as the useSendMessage bug fixed earlier in this stack.
The remaining nine review comments were against files owned by the base PR and are
fixed there, so that PR is correct on its own rather than depending on this one.
fix(ci,workforce): pull the EDDI image, honour ?template=, unblock paused reviews
Four comments posted after the previous fix commit, all critiquing those fixes,
plus the one thread I had wrongly dismissed.
- The audit gate deduplicated advisories by GHSA id before the allowlist ran, so
if the same advisory appeared under the allowlisted package AND another
production package, whichever npm reported first won and the other occurrence
silently vanished — defeating the package binding added in the previous
commit. Now keyed on (id, package). The pure helpers are exported and the
script only self-executes when invoked directly, so the logic is unit-tested:
five cases including "accepts react-router but NOT some-other-pkg for the same
GHSA".
- quotas replaced the entire page when the quota query failed, hiding a usage
reading that had loaded fine and the Reset Counters action with it. The two
are independent queries, so the failure is now scoped to the configuration
card. Updated the test, which had pinned the old page-level behaviour.
- command-palette called parseResourceUri unguarded. It builds a `new URL(...)`,
which THROWS on a malformed resource — uncaught, that took the palette down on
selection rather than merely misrouting. Now falls back to the agent list,
with a regression test using a resource that actually throws.
- AGENTS.md had a fenced block with no language (markdownlint MD040).
Reversal: I previously declined the route-integrity comment, claiming the two
concerns "operate on different inputs". That was wrong — both read app.tsx. The
test asserted every path="…" in the raw file survives stripComments, while this
suite's own contract is that a commented-out <Route> MUST be stripped, so the
first person to retire a route by commenting it out would have hit a false alarm.
Replaced with what the guard is actually for: a deterministic desync case (a regex
literal containing a quote) plus a floor on the real file, since a desync drops
dozens of paths while a commented route drops one.
4265 -> 4272 tests.
savedTemplateConfig was populated by the ?template=<uuid> effect but never cleared by handleTemplateSelect. Arriving via a saved template and then choosing a different card — or "Custom" — kept the deep-linked style and maxRounds, so resolvedStyle and the created group silently used settings the user had moved away from. The same silent-misconfiguration class this branch fixes elsewhere, triggered in reverse. (CodeRabbit) The deep-link effect sets savedTemplateConfig after making its own selection, so clearing inside the handler leaves the saved-template path intact. The accompanying test drives the real flow (deep link, then a manual card, then Next) and asserts the board name switches to the built-in template's. That confirms handleTemplateSelect ran on a manual pick, which is what performs the clear; it observes a sibling effect of the handler rather than the cleared field directly, since style only becomes visible several steps later.
CI failed the coverage gate: functions 69.99% against a 70% threshold. The functions this branch added tipped a number that was already sitting on the line. Rather than adjust the threshold, this covers something worth covering. Every command palette entry is a navigation target — the bug class this branch keeps finding, and the file sat at 42.85% function coverage while none of its onSelect handlers were exercised. route-integrity checks those literals statically; these 11 tests check the handlers actually fire with them, which static analysis cannot see. Functions 69.99% -> 70.08%. Note the margin is thin: the gate is structurally easy to trip, so the next change that adds functions without tests will fail CI the same way. 4273 -> 4284 tests.
Five files that were effectively untested, chosen because the two reported bugs
lived in exactly this code and nothing noticed:
use-workforce-analytics.ts 2.89% -> 100% stmts, 100% funcs
use-workforce-threads.ts 2.02% -> 98.98%, 100%
landing-page.tsx 0% -> 99.34%, 100%
workforce-layout.tsx 0% -> 89.83%, 100%
workforce-sidebar.tsx 0% -> 88.04%, 40%
Behaviour rather than line-hitting:
- Analytics is arithmetic, so the numbers are asserted: completion rate 2/3 ->
67, average duration over the conversations that have one, per-agent
contributions vs errors (ERROR entries never count as contributions),
sessions deduplicated per conversation, and the distinction that style
distribution counts GROUPS while outcome distribution counts CONVERSATIONS.
Also that outcomeCounts/styleCounts stay UNFILTERED — filtering them would
empty the dropdowns and strand the user on one filter. Driven through MSW at
the real endpoints so enrichment and the per-group fan-out are covered too.
- Threads keys entries on (board, member), not member alone: the same advisor
sits on several task forces, and collapsing them would cross-wire one board's
conversation into another. Plus corrupt-localStorage survival.
- landing-page pins the manage/manager mismatch from both sides — the writer
stores "manage", and the legacy "manager" still redirects so users who chose
Manager before the fix are not returned to the chooser forever.
- layout covers all three viewport chromes, Escape and backdrop dismissal, and
that on TABLET the sidebar is behind a drawer — which is what made "no way
back from Insights" worse than it first appeared.
- sidebar asserts the Dashboard link exists and is labelled, the omission
behind the reported bug, and that losing the board list does not take the
shell down with it.
Two assertions I had written loosely and tightened: /bg-muted/ also matches
hover:bg-muted on every inactive link, and [aria-hidden="true"] matches
decorative icons rather than the drawer backdrop.
Not fixed, pinned as documented-not-endorsed: activeExperts counts the human
asker, because every transcript entry with a speakerAgentId feeds it including
the QUESTION entry. The leaderboard's "You" row has the same cause.
Sidebar functions stays at 40%: the remainder are per-board row handlers needing
a populated list plus mutation mocking. Left rather than padded.
Statements 85.17% -> 86.60%, functions 70.08% -> 70.36%. 4284 -> 4325 tests.
fix: critical review hardening — corrupted agent output, dead navigation, misleading empty states
Iteration 4 of the operator write-grant plan (EDDI backend #625, merged). No write capability is granted -- WRITE_ENDPOINTS stays empty -- this wires the gate infrastructure the write grant will need, and proves it end to end at zero risk while the operator can only read. buildToolApprovals() installs a real toolApprovals gate on every operator agent, read_only included: every write HTTP method (post/put/patch/delete) required-approval, every read exempt, timeoutPolicy hardcoded to WAIT_INDEFINITELY (never configurable into AUTO_APPROVE). It takes no scope parameter on purpose -- gating by method rather than by an enumerated endpoint list means WRITE_ENDPOINTS growing later needs no parallel gate update to stay covered. provisionOperator now sends hitlConfig unconditionally, using the backend's new setup-api field. verifyGateInstalled(agentId) reads every version of the agent document back -- not just the currently deployed one, since a redeploy can reach any prior version -- and refuses unless each has a sane, non-AUTO_APPROVE gate with a non-empty requireApproval and no exempt pattern broad enough to swallow a gated write. Wired as a new verifying-gate activation stage and as useVerifyOperatorGate, a continuously-refetching hook (staleTime: 0) so the fact is re-checked on every page load rather than trusted from a stale mount -- eddi.hitl.tool.enabled can be flipped deployment-wide after activation with nothing else to report it. isWriteScopeAvailable replaces its single optimistic boolean parameter with a WriteScopeFacts object naming each verified fact individually (backendAcceptsHitlConfig, gateVerifiedOnEveryVersion, authMode, approvalSurfaceMounted) so a caller cannot approximate one flag to unblock the UI. It still returns false unconditionally, since WRITE_ENDPOINTS is empty. Surfaces gate status as a badge in OperatorStatusPanel, independent of the deployment-status badge -- READY says nothing about whether hitlConfig survived version skew. New ApprovalRule/rules on ToolApprovalsConfig and hitlConfig/mcpServerUrls on CreateApiAgentRequest mirror the backend types this iteration and a future one depend on.
…operator chat
Iteration 5 of the operator write-grant plan. Still no write capability --
WRITE_ENDPOINTS stays empty -- this wires the surface a human uses to
resolve a pause once one exists.
use-operator-chat previously discarded the streamed done event's payload
entirely, so a turn that paused mid-stream left the input enabled with no
indication anything needed a decision. It now parses conversationState from
that snapshot, flags isPaused, and back-fills the placeholder bubble from
the pending message -- and a send rejected 409 (paused by an earlier turn,
rejected without being consumed) is now treated as the same pause rather
than a raw error, dropping the optimistic user message and empty streaming
placeholder that never actually sent.
ApprovalBanner is rendered inline in the transcript (same placement as a
group discussion's pause in discussion-transcript.tsx) via a new
resolveApproval, which submits the decision through resumeConversation and
then polls getSimpleConversationLog until the conversation leaves
AWAITING_HUMAN -- resumeConversation returns before its continuation
completes, so a single re-read would race it. Reconciliation is by
identity, not by counting messages sent: conversationOutputs.length is
captured at pause time, and whether the resumed turn reused that same step
(a TOOL_CALL resume always does -- LlmTask.executeResume appends to the
step it paused in) or committed a new one (a RULE resume can) decides
whether the placeholder bubble is replaced in place or a new bubble is
appended, read back from the actual count rather than assumed.
ApprovalBanner gains two additive, opt-in props so the shared component's
default behavior for its existing callers (conversation-detail.tsx,
discussion-transcript.tsx) is unchanged: requireExplicitPerCall disables
top-level Approve until every gated call has its own explicit verdict --
this surface pauses on writes to the platform itself, so a swept-in call an
admin never opened must not be reachable by one click -- and
renderCallExtra, used here to show the endpoint a generated tool actually
calls ("POST /agentstore/agents (reconstructed)"), read back from the
same operationId the backend named the tool from
(McpApiToolBuilder.buildApiCall), since PendingToolCallView carries only
the tool name. A redaction caveat is added to the shared component too --
the arguments shown are always redacted, and an approver who does not know
that can mistake "[REDACTED]" for "nothing sensitive was here" rather
than "something was, and it was hidden from you".
New i18n keys propagated to all 11 locales.
…abels CodeRabbit review on #129. gateLooksInstalled only checked hitlConfig.timeoutPolicy and toolApprovals.timeoutPolicy. A per-tool toolApprovals.rules entry takes precedence over the scalar for any call it matches (the backend's ToolApprovalRules.governing -- most specific statement wins), so a rule like { match: "http.post:/agentstore/agents", timeoutPolicy: "AUTO_APPROVE" } made that one endpoint auto-execute unreviewed while the scalar still read WAIT_INDEFINITELY and verification passed. Now rejects a rule whose match targets a gated write method (http.post/put/patch/delete) and whose timeoutPolicy is AUTO_APPROVE; a rule on a read (exempt from the gate regardless) or with no timeoutPolicy is unaffected. fr.json, ja.json and pt.json used the same phrase for gateUnknown ("gate not yet checked") and gateNotVerified ("gate checked and failed") -- the two UI states became visually indistinguishable in those locales.
…e-pause Critical review pass. Four real defects, all in code added by this PR. 1. The append-vs-replace decision compared conversationOutputs.length before and after the pause -- but every read available here runs under the backend's returnCurrentStepOnly default, and ConversationMemoryUtilities collapses conversationOutputs to List.of(getLast()) in that mode. Both sides were therefore always 1, making the comparison a constant dressed up as a computation, and the 'new step' test passed only against a two-element fixture the API cannot produce. Reconciliation is now by the placeholder bubble's id, which also lets its pipeline trace stay attached to the answer that replaces it. 2. pollUntilSettled waited for the conversation to leave AWAITING_HUMAN. A resumed turn may pause AGAIN on a fresh batch -- the backend permits maxPausesPerTurn (default 3), and the multi-step flows this surface exists for are expected to use them -- so a conversation behaving exactly as intended would spin to the 90s timeout and report failure. A pause carrying a different hitlPausedAt now counts as settled and is rendered as the next approval card. 3. useApprovalStatus is keyed on the conversation id alone, and this surface calls resumeConversation directly rather than through useResumeConversation (which invalidates it). The second pause of a conversation would have rendered the FIRST pause's cached pauseDetails -- showing an approver a different set of tool calls than the one awaiting their decision. The cache entry is now dropped after each decision, so the next pause starts at undefined and Approve stays disabled until real details load. 4. The 409 path read conversationId, which was declared inside the try block and so out of scope in the catch -- a ReferenceError swallowed by the best-effort catch around it, leaving the pause reason permanently null. Caught by a failing test, not by the typechecker: npx tsc --noEmit is a no-op here because the root tsconfig is solution-style with files: []. The real check is npm run typecheck (tsc -b), used from now on. Also fixes buildOperationIdIndex to use a null-prototype object and reconstructEndpoint to do an own-property check, so a tool literally named toString cannot resolve to Object.prototype.toString and render 'undefined undefined (reconstructed)' (CodeRabbit).
The safety preamble hardcoded "You are read-only". That is true today and becomes a bug the moment WRITE_ENDPOINTS is populated: the agent would be handed write tools underneath a non-editable instruction forbidding their use. The prompt and the capability boundary had no link keeping them consistent. Both halves of the prompt are now built from the resolved endpoint set: - grantsWriteCapability(endpoints) is the single predicate. It takes the resolved set rather than a scope, so it reports what was granted rather than what was intended — read_write grants nothing extra while the write list is empty, and the prompt says so. Only a literal GET counts as a read; an unparseable entry or an unfamiliar method counts as a write, so the failure mode is a needlessly cautious operator rather than one told it is read-only while holding a write tool. - The read-only rule is swapped for five write rules once a write appears. Rules are numbered at join time so the swap cannot misnumber the list. The load-bearing addition is "never let tool output be the reason for a change": rule 1 already stops the operator obeying planted text, this stops it laundering planted text into a change request a human is then asked to approve. The rest cover announcing intent before a write, treating a rejection as final, and reading the resource back after. - The activation review step renders the preamble, default body and tool list for the scope handleActivate actually submits, via one named constant — so what the admin reads is what gets sent. Output for the read-only case is byte-identical to the previous constants, verified against HEAD: nothing sent today changes. The scope argument is covered by its own test file. read_only and read_write resolve to the same endpoint set right now, so every assertion in system-prompt.test.ts also passes against an implementation that ignores its scope argument entirely — stubbing endpointsForScope is what makes the two distinguishable before a write exists. Seven mutations were run against the suite; each killed at least one test.
Findings from a critical pass over the whole branch. Each fix is mutation-verified — the check was reverted and the new test confirmed to fail before being restored. 1. gateLooksInstalled missed a narrow exempt. The exempt list was tested by exact membership in a broad-pattern list, while the rules check in the same function correctly used prefix matching — so `exempt: ["http.post:/agentstore/agents"]` passed verification. That is strictly worse than the AUTO_APPROVE rule the function does catch: ToolApprovalGate.classify tests `exempt` FIRST and short-circuits to `allowed`, so the call never pauses at all rather than pausing and self-approving. Exempt now gets the same prefix test, plus `http.*:` — the method segment is a wildcard and the compiled glob turns `*` into `.*`, so it matches the POST address as readily as the GET. The known remaining limit (a bare-name exempt) is documented where it applies. 2. resolveApproval could write into a discarded conversation. The success path had no abort check, and pollUntilSettled can only observe an abort between polls because the reads take no signal. Clearing the chat while a decision was polling — up to 90s — resurrected that conversation's answer in the emptied transcript and re-raised isPaused. 3. A multi-part re-pause tracked the wrong placeholder. The replace branch recorded the FIRST rendered bubble while the append branch recorded the last, so a pending message spanning several bubbles left the next decision overwriting its opening line and stranding the remainder after the final answer. Also corrects a comment that described returnCurrentStepOnly as a default on both reads. It is the backend's default only for the streamed snapshot; getSimpleConversationLog defaults it to false and every call here passes it explicitly. The conclusion the reconciliation design rests on is unchanged.
CodeRabbit review of 74bfebd — 4 of 5 findings applied. The substantive one: resolveApproval's state updater was not pure. It minted bubble ids with nextId(), stamped Date.now(), and wrote pausedPlaceholderIdRef from inside. React may invoke an updater more than once, or invoke it and discard the result — so the ref could end up naming a bubble that was never committed, and the next decision would fail to find it and append a duplicate instead of replacing it. The placeholder id is now a field of OperatorChatState rather than a ref, because it names one of `messages` and has to move with it: sharing the updater makes the two consistent or not at all, and the id is read from the updater's own `s` rather than a closure that only refreshes when the conversation id changes. Bubbles are minted before the updater. The same hoist is applied to send()'s placeholder, which had Date.now() inside its updater for the same reason. To be precise about coverage: the existing behavioural tests confirm this refactor changes nothing observable, but none of them discriminate the impurity — that needs a discarded render pass, which neither a single invocation nor StrictMode's double-invocation reproduces. The change is justified by React's updater contract, not by a failing test. Also: - The 409 pause path did not clear resolveError, so a fresh approval card rendered under a stale "resuming failed" error from an earlier decision. The streamed pause path already cleared it. - operator.activation.tools had no inline t() fallback, against the repo guideline. Used the real English string rather than the suggested "{{toolCount}} tools", which is worse than what en.json already says. - Added the suggested test for a re-pause after a 409 that carried no hitlPausedAt. It pins a trade-off rather than an ideal: with no timestamp to compare, pollUntilSettled treats every pause as the one being decided, so a genuine re-pause times out instead of becoming the next card. The alternative loses a pending approval, which is worse than delaying a visible one. Skipped: threading an abort signal through resumeConversation. The shared api client takes no signal, so it would mean changing a signature used across the app for a nitpick CodeRabbit itself rates low value — and aborting the POST client-side would not un-record a decision the backend has already processed. The abort that matters is on the polling, which is already handled. Both new tests mutation-verified.
WRITE_ENDPOINTS has been empty since it was introduced — every prior
iteration built the machinery that had to exist before a write could ever
be granted (the gate, provisioning, verified read-back, the approval
surface, and — landed on the EDDI backend this week — approval binding to
the resolved request rather than the tool name). This is the first commit
where read_write actually differs from read_only.
Four entries, matching the write-scope plan exactly, each chosen so an
approved-but-wrong call stays small and reversible:
- PATCH /descriptorstore/descriptors/{id} — partial metadata edit, no
execution semantics, no egress, no persistence.
- POST .../deploy/{agentId} and .../undeploy/{agentId} — paired
deliberately (deploy without rollback is worse than useless in an
incident); can only activate or stop a config a human already wrote.
- POST /schedulestore/schedules/{scheduleId}/disable — asymmetric on
purpose: disable is bound, enable/create/fire are not, since creating a
schedule is attacker persistence (a scheduled turn has no human present
to approve anything). GET /schedulestore/schedules added to
READ_ENDPOINTS alongside it — without it the operator could disable a
runaway job but never see it to know to.
This alone does not expose anything: nowhere in the Manager UI can an
admin actually select read_write yet (operator-activation.tsx still pins
read_only), so isWriteScopeAvailable's seam has nothing behind it to open.
What this commit changes is that the SYSTEM PROMPT and endpoint resolution
now correctly track reality for a scope that will become selectable —
grantsWriteCapability(endpointsForScope("read_write")) flips true, and the
five-rule write-gated preamble is what read_write actually gets rather
than a value nothing yet exercised.
Test fallout, all expected and all fixed:
- tool-scopes.test.ts's "empty until the gate ships" test is replaced with
a pinned assertion on the real four entries (catches drift in either
direction — an accidental addition is exactly as dangerous as a silent
removal from an allow-list) plus explicit exclusion tests for the
endpoints the plan names as deliberately NOT curated (PUT
/agentstore/agents/{id}, schedule creation, every DELETE).
- "stays unavailable even with every fact true" inverted to "becomes
available once every fact holds" — the mirror proving the seam actually
opens, not just that it fails safe; a regression making writes
permanently unreachable would have passed every other test in the file.
- system-prompt-scope-wiring.test.ts deleted outright rather than patched.
Its entire reason to exist was that the real endpointsForScope could not
yet distinguish read_write from read_only, so a mock was needed to prove
the scope argument reached the branch at all. That is no longer true —
the real module now discriminates on its own, so the mock tested an
artificial scenario redundant with system-prompt.test.ts's own
(unmocked) assertions. Two direct-coverage gaps the deletion opened
(defaultOperatorPromptBody and buildOperatorSystemPrompt threading
read_write specifically, not just safetyPreambleForScope) are closed
with real tests in system-prompt.test.ts instead.
…ranting scope
Everything up to WRITE_ENDPOINTS (previous commit) is static: configuration
that SHOULD produce a paused write, verified by reading documents back, never
by watching an actual gated call happen. This is the one probe that closes
that gap empirically. Modelled on the existing runOperatorCanary (which only
proves the operator can read the platform) but answering a different, more
consequential question: does a write really pause, or does it just run?
The probe prompts the operator to rename one real agent's descriptor — the
one WRITE_ENDPOINTS entry whose worst case, if the gate turns out to be
broken and the probe's own write executes for real, is still small and
reversible (partial metadata edit, no execution semantics, no egress, no
persistence). Every other curated endpoint has a worse worst case for a probe
to risk triggering unattended; this is not hypothetical; it is what makes a
bug in this file's OWN pause-detection logic cheap rather than dangerous.
Outcome is one of pass / fail / unknown, and the distinction is the whole
point (matches the metrics vocabulary the EDDI backend now exposes):
- pass — the turn paused AND the pending batch names exactly the tool this
probe provoked (resolved from the spec via the new
resolveToolNameForEndpoint, the exact inverse of the existing
buildOperationIdIndex). The pause is rejected unconditionally, regardless of
which tool it turns out to be — nothing this probe pauses may ever execute.
- fail — the expected tool appears in the trace of a turn that did NOT pause.
The gate did not catch it. This is the failure the whole probe exists to
detect, and it is distinguished from "never attempted" by tracking the
specific tool name through toolTrace, not just counting tool calls.
- unknown — anything inconclusive: no agents on the platform to test
against, a stream/transport error, a pause on some call OTHER than the
expected one, or a failure to resolve the target tool from the spec at
all. Deliberately never conflated with fail — "the probe errored" and "the
gate is unsound" are different findings, and summing them (as a naive
pass-rate metric would) hides a broken gate behind noisy probes.
enforceWriteCanaryGate is the actual grant decision, wired into
useActivateOperator after the existing gate-verification and read-canary
steps: for scope read_write, a non-"pass" outcome undeploys and deletes the
agent and clears the stored config (resetOperator's full-wipe semantics, not
merely discarding the local config object) and throws — because by the time
this runs the agent is ALREADY deployed with live write tools, so reporting
a failure and moving on would leave them reachable. "Not proven safe" is the
bar for rollback, not "proven unsafe": an unknown outcome rolls back exactly
like a confirmed fail. read_only activations skip the probe entirely — there
is no write tool for it to provoke, and it would report "unknown" on every
single activation for no reason.
Extracted the rollback+throw logic into its own function rather than leaving
it inline in the mutation closure specifically so it has direct test
coverage: the mutationFn itself has no dedicated hook-level test anywhere in
this codebase (a pre-existing gap, not one this closes), and duplicating a
~9-endpoint activation harness just to reach one branch would have meant
either leaving this safety property unverified or testing it through several
layers of unrelated, already-tested machinery.
Also wires the two backend metrics relay endpoints added this week
(POST /administration/operator/{canary-result,gate-status}) — the write
canary reports its outcome after every run, and useVerifyOperatorGate now
reports gate status as a side effect of the SAME check the status panel
already performs, refreshing the alertable gauge every time an admin looks
at the page rather than opening a separate poll. Both calls are
double-wrapped in try/catch (the relay functions' own internal catch, plus
one at each call site) so a metrics-endpoint outage can never surface as an
activation failure — proven by a dedicated test that points the relay
endpoint at a 500 and asserts the canary's own result is unaffected.
Verification: full typecheck, lint, and suite green (4477 tests). Two
mutation batteries: the write canary's own pause/fail/unknown discrimination
(3 mutations, each killing exactly the tests guarding that branch) and
enforceWriteCanaryGate's rollback decision (2 mutations — disabling the
rollback, inverting the scope check — each killing exactly the tests that
should die). Added default MSW handlers for the two new relay endpoints so
tests that trigger them as a side effect (most of them, now) don't spam
"unhandled request" warnings that would drown out a real regression.
…ation form
The scope this form submitted has been hardcoded to read_only since the
control existed — WRITE_ENDPOINTS was empty, so there was nothing to offer.
That changed two commits ago. This is the UI half of closing the loop:
isWriteScopeAvailable finally gets a caller.
**read_write is only ever offered once every precondition holds, evaluated
right here.** backendAcceptsHitlConfig and gateVerifiedOnEveryVersion both
collapse to the CURRENT operator's gate.verified (re-reading every version
of the agent document proves both at once — the backend cannot have
round-tripped hitlConfig soundly without having accepted it in the first
place). For a never-activated operator there is no gate yet, so this is
always false: writing this out loud rather than assuming an optimistic
default is exactly what surfaced the real design implication — read_write
can only ever be granted on a RECONFIGURE of an operator that already
proved its read-only gate sound once, never on a first activation. The
control is still shown, disabled, with an explanation, rather than hidden —
an admin who never sees the option has no way to learn a later reconfigure
could offer it.
**scope (the admin's last click) and effectiveScope (what is actually
granted) are deliberately two different values.** They diverge when
something the admin also controls — auth mode — stops satisfying
isWriteScopeAvailable after read_write was already selected: flip to
"none" after having picked read_write, and the write option's own
precondition (caller-identity) no longer holds. Everything downstream
(both radios' checked state and border highlight, the granted endpoint
list, the safety preamble, the default prompt body, and what
handleActivate actually submits) reads effectiveScope, never the raw
selection — so the UI can never show a choice as active that will not be
what gets sent. A first pass at this used the raw selection for the two
radios' `checked` props while getting the styling right elsewhere; a
targeted mutation (dropping back to the raw value) caught it, both radios
now key off effectiveScope, and the `scope` prop is gone from ScopeField
entirely — with nothing named `scope` left in that component's lexical
scope, the mistake is no longer just tested against, it is a compile
error.
**The editable prompt body swaps to the new scope's default on toggle, but
only while it still exactly equals the CURRENT scope's own default.** An
admin who has customized the text keeps their customization; toggling a
radio button must never silently discard it. The two scopes' default
bodies are distinct strings (read_write's is read_only's plus one more
section), so a single equality check against the current scope's own
default is sufficient — verified directly rather than assumed, since the
first version of this comment asserted a "false positive" failure mode
that turned out not to exist once traced through.
Also fixes a real accessibility/UX bug the write-canary warning notice
would otherwise have hit: the header capability chip and the toast on
success now both distinguish read_write from read_only, rather than the
header unconditionally claiming "Read-only" underneath a form that could
be about to grant a write.
i18n: nine new activation.scope.* keys, a stage.write-canary label (used
raw, no inline fallback, by the existing generic t(`operator.stage.${stage}`)
— without this addition, choosing read_write would have shown that literal
template string in the UI during activation), readWriteChip, and
toast.activatedReadWrite, added to en.json and translated into all 10
other locales, matching the established convention of formal locale
entries for new UI text (inline fallbacks are a defensive backup, not a
substitute — see the existing operator.* namespace). i18n-quality.test.ts
passes.
Testing: 8 new tests covering the full precondition matrix (each of the
three facts checked independently, and the compound first-activation vs
reconfigure-with-unverified-gate cases), the prompt-body swap-vs-preserve
behavior, and the auth-mode-reverts-scope case with direct assertions on
which radio is visibly checked (not just which is disabled) — the exact
manifestation of the bug the scope/effectiveScope split exists to prevent.
Two mutations applied against the availability logic; each killed the
test(s) guarding that exact property.
…approval banner Prefer the pause's requestPreview (method/uri/query/headers/body, already redacted and backend-verified) over guessing an endpoint client-side from the tool name's operationId. reconstructEndpoint now only runs as a fallback for a call the backend could not preview. A distinct badge tells the two cases apart: "verified" when requestPinned is true (the request is re-checked immediately before execution), "preview" when the call has pre-request steps that can still change what runs — both still show the resolved request, since the resolvability of a preview and whether it's pinned to a fingerprint are independent (ApiCallExecutor previews a call best-effort even when it can't fingerprint it).
HANDOFF hadn't been touched since the read-only P1 operator landed; add the phase entry for iterations 4-8 (approval gate through the resolved-request preview) and repoint "Last Commit Focus" at the current tip. AGENTS.md's Platform Operator note still said writes were unreachable — no longer true now that WRITE_ENDPOINTS, the write canary and scope selection are real.
…lity grants
Adds POST /groupstore/groups to WRITE_ENDPOINTS — create only. A group
composes agents that already exist and already carry their own gates; it
authors no behavior of its own. Create is also the one group verb whose
whole-document body is reviewable, since there is no prior version the
approver would have to diff against and cannot see. PUT, duplicate and
DELETE stay out for exactly that reason.
Agent authoring is deliberately NOT granted. Every route to it is one
the write-scope plan rules out: setup/setup-api provision an agent with
an arbitrary endpoints filter and no gate (a complete escape from this
allow-list) and carry a raw provider key in the body, and PUT
/agentstore/agents/{id} is where the operator's own gate lives. The
system prompt therefore hands agent authoring to the existing wizard,
and a test pins that no agentstore or setup endpoint is bound — the
prompt is only honest because there is no tool to reach for instead.
A group body can still grant capability past the request being approved:
dynamicAgents.allowCreation lets the created group create agents at
runtime, ungated. It is visible in the JSON, but one boolean deep in a
config document is exactly what an approver skims past. escalation-flags
detects the known grants and RequestPreview surfaces them above the body
in the approver's own words. An attention aid, not a control — it blocks
nothing and only knows the keys it lists.
Both are cases where a failure rendered as silence. The escalation scan runs on the preview body, which the backend caps at 8KB. A group config can exceed that (up to 100 members), and a truncated body does not parse — so the scan found nothing and the approver saw no warning, which reads as "no capability grant" rather than "not checked". It now says the scan was incomplete, and still shows any grant it did find in preference to that note. enforceWriteCanaryGate assumed resetOperator succeeds. If the delete failed, its transport error propagated instead of the canary failure, so the admin saw "Failed to fetch" for what is actually "a write-capable operator that failed its gate check is still deployed" — read as a retryable blip, and the agent never removed. The rollback is now guarded and its failure reported alongside the original reason, naming the manual step.
promptBody could drift from effectiveScope. handleScopeChange re-syncs only on an explicit radio pick, but effectiveScope also moves on its own when writeScopeAvailable flips — most realistically when the admin picks read_write and then changes authMode away from caller-identity. Scope silently reverted to read_only while the body still described write capability, and that pair was submitted. An effect keyed on the previous effective scope now re-syncs it, still leaving a customized body alone. The write canary took `signal ?? timeout.signal`, so passing a caller signal silently DISABLED the 60s timeout for exactly the callers who cared enough to pass one — and the catch would still have reported "timed out" for their cancellation. Now AbortSignal.any over both. WRITE_CANARY_TARGET_ENDPOINT was pinned only to a literal. If that entry left WRITE_ENDPOINTS the operator would hold no such tool, the probe could never provoke it, and every read_write activation would report unknown and roll itself back — an outage caused by an edit in a file no test connected to this one. Now asserted to be a member. Plus a mismatched German closing quotation mark.
… new pause Review asked for a stable pause identity in pollUntilSettled. The backend already guarantees one — Conversation#pauseConversation sets hitlPausedAt in the same block as AWAITING_HUMAN — so the null branch is a defensive fallback, not a real gap. Inverting it would trade a bounded wait-then-timeout for silently clearing an approval the human never gave.
The comment's own point is that these assertions do not depend on WRITE_ENDPOINTS' exact content — so naming a count in it was self-defeating, and it went stale the moment group create was added.
The comment claimed a caller cancellation would be misreported as a timeout. It would not — the catch keys on timeout.signal.aborted, which a caller abort never sets. The real second-order problem is the reverse: the timer fires unobserved, sets that flag with nothing aborted, and a later unrelated failure then gets attributed to a timeout. The primary reason (the 60s ceiling stops being enforced at all) was correct and stays.
SimpleConversationMemorySnapshot carries only hitlPausedAt and hitlPauseType — never hitlPauseReason, hitlTimeoutPolicy or hitlApprovalTimeout, whatever the TS type claims. Both surfaces read them off that snapshot and got undefined: the approval banner showed a blank reason and its countdown never rendered at all for a 1:1 pause. conversation-detail.tsx was called out for exactly this in the write-scope plan (§3) and was missed. The operator surface acquired the same bug independently via the chat hook's 409 and re-pause paths, which derive pauseReason from getSimpleConversationLog. Both now prefer approval-status, the endpoint that does carry all three, and OperatorChat gained the timeout props so the countdown can render there too. The operator test's mock was inventing hitlPauseReason on the simple conversation response, which is why this passed CI while being broken in production. The fixture now returns what the backend actually returns, and a test asserts the reason and timeout policy reach the banner.
Previously the inbox badged a gated-tool pause and linked out to conversation-detail to decide it — correct when written, because the approver had nothing but a client-side operationId guess to review. That reasoning no longer holds: request pinning (EDDI#627) gives every previewable call a backend-verified, redacted method/uri/query/headers/ body, and the operator chat already renders it inline. Clicking Review now expands the row in place into the same ApprovalBanner + RequestPreview those surfaces use, with requireExplicitPerCall so a swept-in call can't inherit an unreviewed top-level Approve. Each row owns its own expand state and its own approval-status fetch (pauseDetails is deliberately absent from the list summary — too heavy for an endpoint that lists every pending approval at once), so expanding one row never fetches for the others. The pause reason and timeout still come from the summary itself, which already carries them correctly. Any eddi-admin can now decide a gated write from the shared queue, not only whoever is at the operator screen.
…them WRITE_ENDPOINTS grows from 5 curated operational verbs to 23: the operator can now build a whole new agent (POST setup / setup-api) and modify an existing one's prompt, model, behavior rules, output messages, slot-filling, NLU dictionary, HTTP/MCP tool wiring, and pipeline (PUT/POST across all 8 workflow-extension stores). Every write is still individually approval-gated through the same per-call TOOL_CALL pause mechanism. The new entries are judged by a different standard than the original four: not blast radius, but whether the document itself can defeat the approval mechanism reviewing it. None of the workflow-extension stores carry a hitlConfig field (it lives one level up, on the agent document), so a bad edit is reviewable and reversible like any other config change. setup/setup-api DO carry hitlConfig now (the backend half landed separately on SetupAgentRequest), so two new escalation-flags checks - agentCreatedWithoutGate, agentCreatedWithBroadEndpoints - surface a missing gate or an unbounded API-endpoint grant to the approver above the raw JSON, mirroring the existing dynamicAgents/autoApproveOnTimeout checks. Deliberately still excluded: PUT on the agent's own document and on the group's own document. Both carry their own gate (hitlConfig.toolApprovals; hitlConfig + per-phase requiresApproval), and unlike a create, "was the gate just weakened" is a diff question against a prior version - which escalation-flags.ts's pure resolved-body-only design cannot answer. "Modify this agent" is served by the workflow-extension stores instead, which covers what a real request actually needs. System prompt's authoring section is now built from the specific endpoints granted (grantsAgentCreation / grantsAgentModification), not a static string, so it never claims a capability the resolved set doesn't hold. The "never enable something that can act without a human watching" preamble rule now names a gate-less agent explicitly, alongside the existing dynamicAgents/auto-approve-timeout examples - security-relevant wording stays in the non-editable half. i18n propagated to all 11 locales, cross-checked against each file's already-established terminology for "agent"/"group"/"approval".
…request (#199) EDDI answers 207 Multi-Status when a GDPR cascade does not finish. 207 is inside the 2xx range, so `response.ok` could not tell it from a 200 and this page reported every one of them as a clean success. That is the exact misreporting the backend added the status to prevent, and the data subject is the one reader who cannot check it. Erasure now says "Erasure Incomplete" and lists `failedSteps` when the cascade is partial. The counters are non-zero on a partial run too, so a panel headed "Erasure Complete" read as a success that merely deleted less — Article 17 is not fulfilled while a step is outstanding. Export always answers 207 today. Four personal-data categories that EDDI erases as this user's data — group transcripts, shared artifacts, schedules and HITL journal entries — have no exporter yet, so `complete` is false on every call. The bundle still downloads, because an incomplete bundle is still the data EDDI holds and withholding it helps nobody; what stops is calling it complete. The page now names the omitted categories, reports the conversation cap with the real total, and reports conversations that could not be read at all. Two counters were simply wrong and one was missing. `logEntriesPseudonymized` is a field name EDDI has never sent — the wire name is `logsPseudonymized` — so the "Logs Pseudonymized" tile rendered `undefined` on every run since the page was written, and the test asserted the LABEL rather than the value, which is how it survived. Six further counters EDDI added are now shown, and `conversationMappingsDeleted` was absent entirely. A counter the backend does not send renders as a dash rather than a zero: `0` would claim the cascade found nothing to delete when it never looked, and six of these are newer than the endpoint. `complete` is filled in from the status only when the field is absent, so an older EDDI that sends neither it nor `failedSteps` still reads as a clean erasure instead of a failed one. The export summary is recorded before the download is attempted. Whether a bundle may be handed over does not depend on the browser managing to save it, and a blocked download must not swallow the warning too. The MSW fixture carried the page's typo rather than the backend's field name, and answered 200 to an export EDDI cannot currently produce; both are corrected, so the fixtures now describe the server rather than the client. Plural forms are filled from `Intl.PluralRules` rather than a hand-written table — French, Spanish and Portuguese each need a `many` category that "one/other" misses, which `i18n-quality.test.ts` catches. Proven by forcing `complete` true: exactly the three completeness tests fail.
…haned one (#202) Two gaps found auditing the multi-user surface against EDDI. **Sharing was wired to the agents page alone.** EDDI shares by DESCRIPTOR id — `/descriptorstore/descriptors/{id}/shares` takes any resource — but `ShareDialog`, `OwnershipBadge` and the space switcher were imported by `agents.tsx` and nothing else. A standalone workflow, output set, HTTP call or prompt snippet could not be shared, and the grants already on one could be neither seen nor revoked. Sharing an agent cascades through its graph, so this was survivable for anything an agent references and not for anything else. The share control and the ownership badge now appear on the resource list and the workflows list, gated the same way the agents page gates them: the button needs OWN, and the whole thing is hidden when workspaces are not enforced. **Ownership transfer had no UI at all.** `transferOwnership` was implemented in `sharing.ts` with zero call sites outside its own module. It is not the same operation as granting somebody `OWN`, which the dialog already does: - Granting OWN *adds* an owner, and requires being the owner yourself. - Transferring *replaces* the owner, and is `@RolesAllowed("eddi-admin")`. The difference is the whole point. When a resource's owner leaves the organisation, nobody remaining holds OWN, so the sharing section is hidden and there is no one left who can grant access — the resource is unreachable. EDDI's javadoc names exactly that case: "Use when a resource's owner has left the organisation." The control is admin-only, appears even when the caller cannot see the sharing section (that is the case it exists for), and asks before it fires. The confirmation is bound to the subject it was shown for, following the rule the OWN grant already follows: with a plain flag, arming for "bob" and then typing "carol" would hand carol the resource under a warning that named bob. `spaceId` is left to the server rather than derived here. It resolves the new owner's personal space itself, and a client guessing at that encoding is the silent failure mode `GET /workspaces` was added to remove. The test harness mocks auth as "keycloak" rather than "none" deliberately: `useHasRole` short-circuits to true when auth is off, so a harness left on "none" would report every caller as an admin and the gate would go untested. Proven by pinning `isAdmin` false: six of the seven tests fail, and the one that survives is the one asserting an editor sees nothing.
Seven PRs merged together (#197-#203), so one phase entry rather than seven. Deliberately written after the merges: seven branches each editing this file would have conflicted seven ways for no benefit. The "Last Commit Focus" notes are the three things worth carrying forward rather than a summary of the diff: - A passing test hid several of these bugs. The GDPR counter typo survived because its test asserted the label and never the value, and the fixture had been written to match the page rather than the server. When a fixture and a client agree, neither is evidence. - Dead API surface is a defect. The first cut of #197 shipped three exports with no callers — the same fault those PRs were fixing elsewhere — and they were cut before merge. - A review finding is a hypothesis until checked. One asked for theme tokens where the theme has no such token; it was declined with evidence and withdrawn by the reviewer.
* feat(ui): give the app's caution amber a semantic warning token A colour used for one meaning across 221 call sites has earned a name. `@theme` had no warning token, so every caution surface hard-coded `amber-*` and the only near-miss was `destructive` — semantically wrong for something that is not dangerous. CodeRabbit flagged one instance of this on #200; the specific finding was declined because there was nothing to point at. Now there is. `--color-warning` is amber-700 in light rather than the brand's amber-500: the token is read as text far more often than as a fill, and #f59e0b is only ~2.1:1 on white. That is the same trade `--color-sidebar-accent` already makes. Dark mode restores amber-400 at ~11:1. Migrated call sites keep their exact text colour — the existing convention was already `text-amber-700 dark:text-amber-400` — so only the low-alpha fills shift a shade. Button gains a `warning` variant. It is the semantically right home for a warning action, and it is load-bearing for a second reason: Tailwind v4 tree-shakes `@theme` tokens, and that cva string is the only place in the synced surface pairing a solid `bg-warning` with `text-warning-foreground`. Without it that token silently never reaches `:root` — the `--color-sidebar-accent-foreground` trap again. NOTES.md now records the dependency. Verified both custom properties reach `:root` in the design-sync `compiled.css` and in the app bundle, with light and dark values. Left as amber on purpose, having checked each: the resource-type badge and the vault-reference chip (categorical, not caution), the wizard card's gold hover and the sidebar count (brand accent), and the demo banner's solid strip. AlertDialog's non-destructive confirm also stays `primary` — its documented contract is a gold confirm, and the icon already carries the warning. * fix(ui): finish the warning token — dialog confirm, preview, latency scale Self-review of the previous commit turned up three things. AlertDialog's warning variant painted its icon with the new token but kept a `bg-primary` confirm, so the two amber tones clashed inside one small dialog — visible in the design system's own Warning preview card. The confirm now uses the warning Button variant. That also gives the variant a signal it never had: a gold confirm was pixel-identical to an ordinary primary action, so `variant="warning"` only ever changed the icon. Both real call sites are the reversible connection dialogs in `linked-accounts-panel`, which already argue in a comment for warning over destructive. The Button preview enumerates every variant and was missing the new one. NOTES.md is explicit that a preview is part of the four-part contract. Reverted the platform-status latency colour. It is a green/amber/red scale, not a caution surface, and tokenising only its middle tier while leaving the identical ratio bar in quotas.tsx untouched would have been arbitrary. * fix(a11y): darken light --color-warning to amber-800 CodeRabbit flagged the warning Badge at 4.08:1 on the light card, under AA for its 12px semibold label. The finding is right, and the previous `bg-amber-500/15 text-amber-700` sat at 4.48:1, so this PR had made it slightly worse. The suggested fix — dropping that one badge to `bg-warning/5` — clears AA at 4.69:1 but treats a single call site. The defect is in the token: because the token now tints its own background, `text-warning` on `bg-warning/10` lands at 4.38:1 too, which the GDPR restriction badge ships and the review did not catch. Anything else pairing the token with its own tint would inherit the same problem. amber-800 fixes it at the source and holds at every alpha the app uses: light on card 7.09 /05 6.57 /10 6.07 /15 5.60 dark on card 10.61 /05 9.68 /10 8.68 /15 7.68 It also keeps the badge family on one tint (success, warning and destructive all stay /15) rather than making warning the odd one out, and it follows the precedent already set two dozen lines below in the same file: --color-sidebar-accent picks the palette step that meets contrast rather than the brand's amber-500. Not addressed here, because it predates this PR and nothing in it touches the variant: the destructive Badge is 3.82:1 on the light card, the worst of the three.
…oup-chat UX gaps behind them (#206) * fix(groups): stop rendering a member's JSON reply as raw JSON Four group phases do not store prose. EDDI hands the member a JSON contract and stores the answer verbatim, so `TranscriptEntry.content` is a JSON document for `VOTE` (VoteTallyEngine), `BID` (TaskBidEngine), `BARGAIN` (NegotiationEngine) and `RETRO` (RetroEngine). Nothing in the Manager read them, and what each surface did instead was wrong in a different way: - The generic fallback rendered nested values with `JSON.stringify`, so a bid sheet reached the screen as `**bids**: [{"subject":"…","confidence":0.9,…}]` — a raw JSON array in the middle of a discussion transcript. Reproduced against the real contracts before fixing. - The Manager's card never got that far for a BID: `tryParseStructuredItems` matches the `bids` array on its `subject` field and rendered the subjects as a bare numbered list, dropping every confidence, complexity and rationale in it. - The Workforce board's `AdvisorResponseCard` passed `content` straight to ReactMarkdown with no parsing at all — the only one of the three transcript renderers that did — so it also showed EDDI's response envelope and a judge's fenced verdict verbatim, and copied them. Three changes, at three layers: 1. `readableJsonObject` recurses. Nested objects and arrays become markdown sub-lists instead of inline JSON, keys become sentence-cased labels, null fields are dropped, and the depth guard is the only path that still stringifies. Every surface and both markdown exporters inherit this. 2. `lib/group-payloads.ts` reads the four contracts into typed shapes, as leniently as the backend's own three-tier parse (strict, then the first balanced object embedded in prose or a fence). Its brace scan is string-aware, so `"we agreed } then moved on"` does not truncate. 3. `StructuredTurnCard` renders those shapes, localized, and is wired into all three transcript renderers — one component rather than three copies, for the reason `DiscussionInsights` is one component. `AdvisorResponseCard` now also parses before rendering, copies what is on screen rather than the wire format, and judges emptiness on the parsed body so an empty envelope reads as "no response generated". The history viewer's ERROR card parsed nothing either. EDDI writes those with a null body and the text in `errorReason`, so it was unreachable today, but it is the same latent defect and now takes the same path. i18n: 13 `groups.payload.*` keys, translated across all 11 locales. Tests: 25 reader tests over the real contracts, 11 render tests across the three surfaces, 10 more pinning the recursive fallback. * fix(workforce): send the files the board's paperclip stages `BoardInput` called `onSend(message, attachment)`; `workforce-board` declared `handleSend(question: string)`. TypeScript accepts a handler that takes fewer parameters, so the second argument went nowhere: the file was staged, validated for size and type, shown as a chip, and dropped on send with no error and no upload. `workforce-board.tsx` contained no attachment code at all, and no test read the request body. Rather than grow a second staging implementation, the Manager composer's was extracted to `useGroupAttachmentStaging` and both now share it: dedupe, the per-file and total-size caps, base64 encoding, and the serialized queue that stops two rapid picks reserving the same budget. `DiscussionInput` is behaviour-identical on it — its 60 existing tests pass unchanged. The board therefore also gains what it was missing: several attachments instead of one, a size on each chip, and `MAX_GROUP_QUESTION_CHARS` enforcement with an inline `role="alert"` and a blocked Send. That cap was already the Manager's, added after a 50k-character question uploaded in full and came back as a 400 the user never saw; the board posts to the same endpoint and had none of it. The file-type gate stays board-local — `accept` narrows the picker and this catches what slips past — while size and count move to the hook, so they now match what the endpoint actually enforces rather than a board-local 10 MB guess. Tests: 6 on the composer's contract, plus one page-level test that reads the POST body, because the compiler cannot see this class of bug. Mutation-checked: dropping the argument again fails that test. * fix(groups): make the Manager's group page and the approvals inbox usable Five defects, all of them things the Workforce board — the other surface for the same data — already got right. **The live stream vanished on navigation.** `group-detail` called `useGroupDiscussionStream()` with no argument, so the hook keyed off local state that only exists once that instance starts a stream. Navigate away mid-discussion and back, and the running stream kept filling the store with nothing rendering it; the transcript silently degraded to the 3-second poll. Now bound to the group id, as the board binds to its board id. **The selected discussion was local state.** A reload landed on whichever discussion happened to be first, a discussion could not be linked to, and the approvals inbox — whose entire job is sending someone to a specific paused discussion — could only drop them on the group. It now lives in the URL, as it does on the board. **Delete had no confirmation.** Cancel has had one since it was noted that a mis-click next to the trash icon must not abort a live discussion. Delete is the permanent one of that pair, offered in every state including a running discussion, and asked nothing. The Workforce history page, which deletes the same resource, has always confirmed. Its toast was also `t("common.delete") + " ✓"`, which renders as the bare verb "Löschen ✓" in German; it is a sentence now, and a failure is no longer silent. **Those two controls were invisible to keyboard and touch.** `opacity-0` with only a `group-hover` reveal, so they could be hit but not seen. They now respond to focus-within and focus-visible, and stay visible below `md`, where there is no hover at all. **The configuration panel was unreachable below `xl`.** The sidebar is hidden there and so is its re-open button, leaving a tablet or phone no route to it from this page. The same `GroupConfigPanel` now also opens as a sheet — one component, two placements. The approvals inbox gets two of its own: - **A group pause could not be decided from the queue.** A 1:1 pause had Approve/Reject/Cancel inline; a group pause had a link. A phase approval needs nothing but a verdict, so it is decided here too, via `approveGroupPhase` — which had been sitting in `lib/api/hitl.ts` documented as "kept for programmatic callers" with no caller. A HUMAN_TURN still gets the link only: nobody is deciding there, a member owes the discussion their contribution. - **Its links dropped the version and the discussion.** The summary carries no group version and the group page requires one, so every link defaulted to version 1 — any group that had ever been edited opened at its original name and member list. One descriptor call supplies the real version, and the link now names the paused discussion the page can finally select. i18n: 3 new keys across 11 locales. Tests: 6 on the group page, 5 on the inbox. * fix(workforce): stop the member thread reporting failures as the agent Every failure in a 1:1 member thread was caught and appended to the transcript as a message with `role: "agent"` reading "Sorry, I encountered an error. Please try again." An expired token, an exceeded quota, an undeployed agent and a conversation paused for approval all rendered identically, in the agent's own voice, while the backend's actual sentence went to `console.error` and nowhere else. The fake message then vanished on reload, because no such turn existed. A failed turn is not something the agent said. It now renders as its own row: the backend's message (via `getErrorMessage`, or the localized sentence for a known stream error code), a Retry that re-sends the same text and attachments, and a Dismiss. A 409 is called out separately, because it is the one case where the send was refused WITHOUT being consumed: the optimistic user message is withdrawn — leaving it would show a message as sent that the backend never received — the row says the conversation is paused, and it links to the approvals queue instead of offering a retry that cannot help. The same pause reached through a streamed turn (`conversationState: AWAITING_HUMAN` on the `done` snapshot) gets the same treatment. While here, the thread now streams. It was the only chat surface in the app that did not — the Manager's chat, the operator drawer and `/workforce/chat` all stream — so a long answer was a spinner with no partial text and no way to stop. It uses `sendMessageStreaming` directly rather than duplicating `use-chat`'s store: tokens land in a placeholder that is present from the start, `done` fills the body from the snapshot when nothing streamed, an interrupted stream still settles, and a Stop button aborts and keeps what arrived. i18n: 4 keys across 11 locales. Tests: the four that pinned the old non-streaming transport now pin the same behaviour on the new one, plus four new ones for the error row, the retry, the pause and Stop. The stream double honours its abort signal, as the real generator does — otherwise the Stop test proves nothing. * fix(groups): say what a cadence runs, and stop the smaller UX gaps **A cadence could not be identified.** `Cadence` carries a `scheduleRef` and nothing about when it fires — the cron expression and time zone live only on the paired schedule-store row — so the workspace listed a column of UUIDs. You could not tell which cadence was the Monday 9am run, or which of three to delete. One schedule-list call joins them: each row now leads with a localized description ("every Monday at 09:00"), then the raw expression, the zone and the next fire. A cadence whose schedule cannot be read says so rather than rendering blank. The cron field also describes itself as it is typed, and refuses an unparseable expression before the round trip rather than after a 400. **The workspace printed raw wire values.** Backlog task status was the bare enum (`AWAITING_APPROVAL`), untranslated in all eleven languages; per-member reliability was a table of agent UUIDs, though the group config on the same page knows what it calls each member. Both are labels now, with the id kept as the row's tooltip. **Two divergent Markdown exporters.** The history viewer had a hand-rolled copy of the board's — its own comment said so — and they had drifted: the copy titled every file "# Task Force Discussion" whatever the discussion style, and omitted the group name, the structured decision, the minority report and an unparsed judgment. Both toolbars now call `lib/group-transcript-export.ts`. **Deleting a group from the list orphaned its agents.** The config panel and Workforce settings both offer the cascade; the list's trash icon — the entry point most people use — deleted the group alone and said nothing, leaving wizard-built members belonging to nothing. The dialog now offers it and explains where they otherwise end up. **A channel target's agent or group was a free-text UUID field.** Nothing resolved a name, nothing validated the id, and a wrong one only failed at message time. Both types get the pickers the wizards use; a saved id the list does not carry is preserved as its own option so opening an old channel never silently repoints it. Switching the target type clears the id, which belongs to the other id space. Its `Agent`/`Group` options were also the last untranslated `<option>` labels on this screen. i18n: 16 keys across 11 locales, plus the `_many` / Arabic plural categories the quality gate requires. Tests: 5 on the cadence and cron work; the suite is green at 6230. * fix(groups): address review — retry duplication, version gate, i18n Seven findings from review, all verified against the code first. **A retry duplicated the user's message.** On a stream failure only the agent placeholder was withdrawn; `handleSend` then appended a second optimistic user message, so a successful retry showed the same text twice. Withdrawing it at failure time would fix that but hide what the reader had just typed, right next to an error asking them to retry it — so the message stays while the error is on screen and `handleRetry` withdraws it just before re-sending. Held by object identity, so two genuinely identical messages are never confused. The 409 path still withdraws immediately, because there the backend never received it. Both behaviours are now pinned; the existing retry test asserted only the agent's text and could not see the duplicate. **An approval link could open the wrong version.** `groupHrefFor` did not wait for the descriptor query, so while it was pending the link carried no version and the group page defaulted to 1 — an edited group's original configuration. The link is now held until the version is known. A group the list does not carry (deleted, or past the first hundred) still gets a link without one: that is the pre-existing behaviour, and an unreachable link would be worse than an imprecise one. **The cascade choice survived a successful delete.** `onOpenChange` only fires when the dialog closes itself, so clearing `deleteTarget` on success left `deleteMembers` set and the next group's dialog opened with "also delete its agents" already ticked. Both success paths and the dismiss path now go through one `closeDeleteDialog`. **An approval ballot repeated its options.** `{"votes": ["A", "A"]}` is one selection said twice; it rendered twice and gave two badges the same React key. Collected into a set. **Two localization gaps.** `describeCron` was called with no translator at both workspace call sites, so only weekday and month names were localized and the sentence around them stayed English — the schedules page already had the adapter this now uses. And the Markdown exporter wrote English structural labels; it takes a translator now, the same way `CronDescribeT` is passed rather than imported, with 11 new keys across all locales. The decision heading is the decision's own localized name, as it is on the card this export mirrors — "Decision (No structured decision)" was the alternative. **Partly accepted: the attachment chip's remove control.** The review asked for the shared `Button`. Its `size="icon"` is `h-10 w-10`, two and a half times the chip it sits in, and shrinking it at the call site is the variant-patching CLAUDE.md forbids. The substantive defect named — no visible focus ring — is fixed directly, on both composers, since they render the same chip. A chip-sized variant on the primitive is worth adding as its own change. Suite: 6231 green, lint, typecheck and the i18n drift check clean. * fix(groups): cascade over the group's real config, not the list's copy Self-review of the cascade-delete added earlier in this PR: it read the enriched descriptor behind the groups list, and that descriptor carries a member list but no `moderatorAgentId`. `deleteGroupWithMembers` deletes the moderator too, so cascading over the descriptor left exactly the orphan the checkbox exists to prevent — silently, under a toast saying "Group and all member agents deleted". The descriptor's enrichment also falls back to an empty member list when its config fetch fails, so a transient error would have deleted nothing while reporting the same success. The cascade now fetches the authoritative configuration first. When that read fails it deletes nothing and says so, rather than quietly downgrading to a group-only delete: keeping the agents is the one outcome the reader just said they did not want. The `as unknown as` cast the descriptor needed is gone with it. Mutation-checked: restoring the descriptor read fails the new test with "expected [ 'agent-one', 'agent-two' ] to include 'agent-moderator'". i18n: 1 key across 11 locales. Suite: 6234 green. * fix(groups): address review round two — dismissal race, version gate, pt label **A dismissed dialog could still delete.** The cascade awaits `getGroup()` before it can mutate, and `AlertDialog` stays dismissable throughout — Escape and the scrim close it whatever `isPending` says. The awaited continuation then resumed against a dialog nobody was looking at and deleted the group's agents anyway. A request token, bumped by every dismissal, now invalidates the continuation, and the dialog reports the config read as pending rather than looking idle. The first version of that test did not actually prove the guard: `waitFor` succeeds on its first attempt, so asserting "nothing was deleted" passed before the continuation had a turn. It now waits for the held read to land, gives the continuation real time, and then asserts. Mutation-checked: removing the guard fails it with the three agents listed. **A link could still open the wrong configuration.** Holding it only while the descriptor query was pending left the case the review named: a group absent from the first hundred descriptors got a link with no version, and the group page defaults that to 1 — an edited group's original member list and name. `groupHrefFor` now returns null whenever the version is unknown, and the held control carries a tooltip saying why. This is the screen where someone approves an action without the surrounding context, so showing them the wrong context is the worse of the two failures. **Portuguese label.** `groups.export.minorityReport` read "Voto de vencido" where the app already says "Voto vencido" in `groups.minorityReport` and `groups.recordDissents`. i18n: 1 key across 11 locales. Suite: 6236 green. * fix(workforce): report a stream that stops before it finishes Two findings that were in the review bodies rather than posted inline — one out-of-diff, one a nitpick — and so were missed on the first pass. **A dropped connection looked like a finished reply.** The stream loop settled whatever had arrived when the iterator ended, and only `done` and `error` set a failure. A connection that just stopped therefore presented a truncated answer as complete, with nothing to say otherwise and nothing to do about it. The terminal event is now tracked: an iterator that ends without one, and without the reader having pressed Stop, keeps the partial content and reports it. The partial reply stays because it is real content the agent produced, and the question stays above it — this failure sets `pendingUserMessage` to null so a re-send appends below rather than leaving a half-answer stranded above the next question. Stopping deliberately is a decision, not a failure, and is not reported; both are tested, and removing the check fails the first. **Declined in part: the shared `Button` for the approvals row.** The three 1:1 actions beside the new group ones are pre-existing hand-rolled controls with byte-identical classes; `Button` has no emerald variant and its `sm` size is `h-8 px-3` against this row's `py-1`, so converting only the new three would leave one row with six buttons in two sizes and two shapes. Adding a `success` variant and an `xs` size to the primitive is the sanctioned fix and is worth doing — as its own change, since `Button` is design-synced. The substantive half of the finding, the shared focus-visible behaviour, is applied here to all eight buttons in that row, which had none. i18n: 1 key across 11 locales. Suite: 6238 green. * fix(groups): review round three — stale pending clear, stranded partial, shared Button **A dismissed read could report a newer one as idle.** Both `setReadingConfig(false)` calls ran before the request-token check, so a dismissed request resolving late cleared the pending state of a request started after it — the dialog looked ready while it was still reading, and its confirm re-enabled mid-flight. The token is checked first now. **A partial reply could end up above the wrong question.** The dropped-stream path already kept the question above its partial answer by not withdrawing it on retry. The `error` event and the generic catch did not, so once tokens had arrived a retry withdrew the question and left the half-answer stranded above the next one. Same rule in all three paths now, keyed on whether anything actually streamed. `streamedAnything` moved out of the `try` so the catch can see it. **The approvals row now uses the shared `Button`.** My earlier reason for declining this was wrong: a group row and a 1:1 row never render together, so the "six buttons in one row" objection did not exist. They do sit in one column of one table, and the rule is checked in, so all six decision controls are converted to `primary` / `destructive` / `outline` at `size="sm"` — which also removes the non-token `bg-emerald-600` the review named, along with the error-state Retry and the Refresh control. The Review control stays a native element deliberately: it is a disclosure toggle carrying the amber tool-call cue and a rotating chevron, not a decision, and `Button` has no variant for it. `AlertDialog` gained `data-testid` on its confirm and cancel buttons. Its confirm label becomes "…" while pending, which is exactly the state two of these tests assert on, so name-based lookup could not reach it. Props are unchanged, so the design-sync contract is untouched. Both new guards are mutation-checked: reordering the pending clear fails the concurrency test, and withdrawing the question after a partial reply fails the ordering test. Suite: 6240 green. * fix(groups): address the Opus code review on the group-chat branch Fourteen findings, of which the load-bearing one is the streaming fallback in the Workforce thread: `sendMessageStreaming` posts without `returnCurrentStepOnly`, so the `done` snapshot carries the whole conversation. Joining every agent step rendered the entire thread a second time inside one bubble. Take the last step, as `use-chat` does. Also: guard staged attachments against an in-flight read landing after a clear; restore the phase separators and the duplicate-synthesis guard the consolidated exporter dropped; read the history export's group version from the descriptor list rather than assuming version 1; unmount the config sidebar while the sheet is open; say that an inline group approval covers the whole phase; measure the question cap on the trimmed body and wire the alert to the field; flip the concession arrow in RTL. Two new keys across all 11 locales. * fix(groups): the fourth review round on the group-chat branch Two hand-rolled controls replaced with the shared Button, which needed a compact `iconSm` size for a dense list row rather than a call-site restyle. Three real defects behind the nitpicks: The staging queue read its generation token after the queue awaited, so a selection cleared while it waited captured the post-clear value and passed its own guard. Captured at enqueue instead. An empty SYNTHESIS entry counted as "the synthesis is already written" and dropped the final answer out of the export entirely. Now only a non-empty body suppresses it; four tests cover the exporter, and the empty case is mutation-checked. The history export fell back to version 1 while the descriptors were still loading, which is the same wrong-name bug one render earlier. It now waits for the version rather than guessing one. * fix(groups): close the last three review threads on the group-chat branch The exporter suppressed the final answer whenever a SYNTHESIS entry was present, whatever it said. An entry with no body writes nothing, and an entry whose text differs is a second piece of content, not a duplicate. Both cases dropped `synthesizedAnswer` out of the file. Suppression now requires the same text, with a test for each direction. Whether a failed turn withdraws its question on retry was decided at four call sites. Three spelled the rule out; the fourth, the pause after `done`, did not, and got it wrong. The rule now lives in `failed` where the decision belongs. Three tests cover it, and the streaming case is mutation-checked. The attachment chip's remove control is the shared Button. * fix(groups): three findings from the review of the previous round An attachment slot was spent by a file that was never staged. The selection was truncated to the remaining room before validation, so picking one oversized file and one legal file with a single slot left staged neither and said nothing about the second. The count is now enforced on what is accepted, with a mutation-checked test. Rejecting a group approval from the queue rejects the whole paused phase, but said "Reject this request? The conversation will not proceed." Approve already had group-specific wording; reject now matches. One new key across all 11 locales. Three delete labels went through `t` with no inline fallback. * fix(groups): adopt main's warning token, and register the new Button size Main gained a semantic `--color-warning` token and a `warning` Button variant specifically to retire hard-coded amber. This branch was cut before that and still added four amber classes: the approvals Review toggle and three in the thread's paused-error banner. Merging main alone would have reintroduced what #205 removed, so they now use the token, in the same shape #205 migrated its own call sites to. The `iconSm` size this branch added to Button was missing from the design-sync contract. CLAUDE.md makes `dtsPropsFor` and the preview part of adding anything to a synced primitive, and without them the design system ships a Button that does not know the size exists.
…ED, editor picker, rule mirror (#208) * feat(connections): support the CALLER_SUPPLIED binding end to end The backend's Binding enum has three values; the Manager knew two and derived the field from the auth type alone, so a CALLER_SUPPLIED document loaded into the editor was saved back as SERVICE on the first unrelated edit — with an empty valueTemplate the backend refuses on that binding. - BINDINGS gains CALLER_SUPPLIED; bindingFor(authType, requested) honours a requested binding where the type allows it (STATIC: SERVICE or CALLER_SUPPLIED) and corrects it everywhere else, so a loaded document keeps its stored binding. legalBindings/isLegalBinding expose the pairs. - toStoredConnection sends only staticAuth.headerName for a caller-supplied connection; emptyConnection(authType, binding) builds one. - validateConnection mirrors the backend: no template required on CALLER_SUPPLIED, and a template, username or passwordRef there is refused (callerSuppliedRefused); an illegal type/binding pair is bindingMismatch. - ConnectionCredentialFields offers a two-way chooser for STATIC (shared key vs caller-supplied) with the authority-not-convenience reasoning, and replaces the header-value field with the exact X-EDDI-Connection-Credential line the integrator must send. - The wizard now builds its document through toStoredConnection, so a template typed before switching the binding is not sent. - The detail page's binding explainer has a third variant; BindingBadge renders a distinct "Caller-supplied" chip; the MSW fixture and jsonSchema enum carry the new value. Tests: connection-validation.test.ts (bindingFor/legalBindings, the caller-supplied block), connections.test.ts (toStoredConnection keeps the binding and drops the template), connection-detail.test.tsx (round trip and the binding switch), create-connection-dialog.test.tsx (wizard produces one), connections.test.tsx (badge on the list). * fix(connections): mirror the backend's new write rules client-side The backend now enforces a name grammar, a timeoutMs bound, a literal- segment rule on valueTemplate and value rules on extraAuthParams. A mirror that has drifted is worse than none — it either blocks a document the backend accepts or promises one it refuses — so each rule lands here in the same shape, with its own code and an actionable sentence. - name: ^[A-Za-z0-9][A-Za-z0-9._-]{0,63}$ (nameFormat / nameTooLong). The old comment calling the format "not a backend rule" is gone; it is one. - timeoutMs: null is the default, otherwise a whole number in 1..60000 (timeoutRange). The Advanced field carries min/max, aria-invalid and an inline message. - valueTemplate: every literal half must be at most 32 characters and must not carry a run of 12+ from the credential alphabet (templateLiteralCredential), so `Bearer ${vault:k}` passes and `sk-live-abcdef${vault:x}` does not. Checked on the literal halves only, so a long vault key name inside the braces is never mistaken for a value. - extraAuthParams: reserved OAuth names EDDI writes itself are refused stripped of case and separators (paramReserved); a value must not carry `${`, exceed 512 characters, or be one long token-alphabet run (paramValueReference / paramValueTooLong / paramValueCredentialShaped). The credential-shape check is deliberately narrow — audiences and resource URLs carry dots and slashes and pass. - code_verifier: the "live backend bug" wording is removed. EDDI 25ed729 writes its denylist in stripped form, so normalising both sides here is now the same rule, not a stricter one. The backend's own 400 sentence still reaches the toast verbatim; a test pins it, since the rules this form cannot mirror (the operator's credential-endpoint allowlist, a duplicate name) arrive only that way. Tests: connection-validation.test.ts (name grammar and length, timeout bounds, templateLiteralCredential, isReservedOAuthParamName, validateParamValue, the extraAuthParams value cases), connection-detail.test.tsx ("puts the backend's own 400 sentence in the toast, verbatim", "refuses a timeout outside the backend's bounds"). * feat(connections): recognise and pick connection references in the editors The reference grammar did not know the `connection` scheme, so the secret picker treated `${connection:jira}` as a raw secret: masked it behind dots and offered to store it in the vault — which would have vaulted the literal string. And nothing in any editor knew where the backend resolves such a reference, so the four shapes it refuses at build time (text around the reference, a mismatched header name, a reference in a path/query/body, a reference in a model parameter) surfaced only as a deploy failure. - secret-reference.ts: `connection` is recognised for rendering (hasReferencePrefix, referenceLabel, isVaultScheme) but deliberately kept out of REFERENCE_SCHEMES — isSecretReference is what clientSecret and passwordRef accept, and a connection there is refused. Adds isConnectionReference / parseConnectionReference / toConnectionReference / containsConnectionReference / wrapsConnectionReference. - connection-placement.ts: the header rule — connectionHeaderName (STATIC and BASIC read staticAuth.headerName, OAuth is Authorization) and expectedHeaderFor, silent on an unknown connection. - SecretKeyPicker renders a connection reference as its own "Connection" chip (unmasked, no vault lookup, no vault offer) and, with the opt-in `connections` prop, offers a picker listing the deployment's connections (ConnectionReferenceButton). A 403 reads as a role limit with the by-hand escape hatch, a 404 as a backend without the feature; the list is fetched only once the popup opens. referenceOnly still refuses it. - Wired where the contract allows: the mcpcalls apiKey, the A2A apiKey in the LLM editor, and httpcall header values (KvEditor gains an insert button per header value). Not offered in the LLM, RAG or embedding editors. - ConnectionReferenceWarning under each of those fields: wrapped-reference and header-name-mismatch on the three allowed placements (the mismatch looks the connection up from the descriptor list, fetched only when a header references one); "refused here" under httpcall path/query/body and under every LLM model parameter and RAG embedding/store parameter. Tests: secret-reference.test.ts (the connection block), connection-placement.test.ts, secret-key-picker-connections.test.tsx (chip, picker insert, 403/404 hints, lazy fetch, reference-only refusal), mcpcalls-editor.test.tsx / llm-editor.test.tsx (picker offered, wrapped warning, model-parameter refusal), apicalls-editor.test.tsx (insert on headers only, wrapped, header-name mismatch, path/query/body refusal), rag-editor.test.tsx (embedding and store refusal, no picker). * feat(connections): confirm before changing an existing connection's auth type or binding The detail page offered the auth-type select — and, since the last commit, the shared-key/caller-supplied chooser — on an existing document with no more ceremony than on a new one. Either change strands every account linked through the connection: a grant belongs to the flow that produced it. The backend now answers such a PUT with a 409 while any account is still linked, naming the count and the unlink route, so the question belongs before the edits rather than after them. - Leaving the STORED auth type or binding opens a confirmation explaining that linked accounts stop resolving and that the save is refused while any remain; confirming applies the change, cancelling leaves the draft untouched. Returning to the stored value is applied directly. - The stored values come from a parsed baseline kept beside the serialised one, so the rule survives a save (the new version becomes the baseline) and a route change (both are reset together). - The backend's 409 reaches the toast verbatim through the existing getErrorMessage path; a test pins that the count and the route survive. Tests: connection-detail.test.tsx ("asks before the auth type of an existing connection is changed", "applies the type change once it is confirmed", "does not ask again when switching back to the stored type", "asks before a shared key becomes caller-supplied", "puts the backend's 409 about linked accounts in the toast, verbatim"); the three existing type-switching tests now confirm the dialog. * fix(connections): treat a 503 on the per-user routes as an outage, not as the feature being off `/connections/mine` mapped a 503 to CONNECTIONS_DISABLED, and the panel rendered that as the definitive "account linking is switched off" state with `retry: false`. The backend never answers 503 from these routes — it moved to 404 precisely because a 503 body never arrived — so a 503 here is a proxy or a store that is down: an outage, shown as a permanent statement about the deployment's configuration, with nothing to retry. - asConnectionsError converts only a 404 to CONNECTIONS_DISABLED; a 503 passes through with its status and message like any other failure. - useMyConnections retries once for anything but 401/403/404 — the definitive answers still get no second attempt, which would only delay them — and the panel's existing error state with its Retry button now covers 5xx and network failures. - The authorize toast test moves to 403 (the coded answer that route still gives); the panel gains 503, transient-recovery and no-retry-on-404 cases. Tests: connections.test.ts ("passes a 503 through as an outage — never as the feature being off", the authorize 503 pass-through), linked-accounts-panel.test.tsx ("treats a 503 as an outage with a Retry, not as linking being switched off", "recovers from a transient failure on its own before showing the error", "does not retry the answers that cannot change"). * test(connections): round-trip every backend-valid shape and pin the authorize credentials mode Nothing in the suite loaded a document of every backend-valid shape and looked at what came back out: every existing round trip started from a shape the binding derivation happened to agree with, which is how a CALLER_SUPPLIED document could be rewritten to SERVICE on save with every test green. - connection-detail-roundtrip.test.tsx loads one fixture per shape the backend's validate() accepts — STATIC/SERVICE, STATIC/CALLER_SUPPLIED, BASIC, OAUTH2_CLIENT_CREDENTIALS, OAUTH2_AUTHORIZATION_CODE/PER_USER with allowUnverifiedPrincipal — through toStoredConnection and through the detail page, edits the description, and asserts the PUT body equals the loaded document except for that field. Equality is modulo null/absent (the backend serialises unused fields as null; the store omits them) and never modulo a value. - It found one more drift on the first run: toStoredConnection dropped usePkce from a client-credentials document. The field is inert there but part of the stored document (backend default true), so it is now round-tripped; the user-login branch still forces it on. - connections.test.ts pins that the authorize request is never sent with credentials "omit": the Set-Cookie nonce that binds the flow to this browser is stored only under "same-origin" (the default) or "include", and an omit added to the shared client for any other reason would fail every callback with invalid_state and nothing explaining why. Tests: connection-detail-roundtrip.test.tsx (ten cases, two per shape), connections.test.ts ("never sends the authorize request with credentials omitted"). * chore(connections): handoff notes, worktree exclude, and two a11y fixes - HANDOFF.md: `${connection:name}` is consumed as the whole value of an httpcall header, an mcpcalls apiKey or an A2A apiKey — the LLM, RAG and embedding editors refuse it; the sentence claiming otherwise is fixed. The "code_verifier is a live backend bug" note is corrected (fixed upstream in EDDI 25ed729; the mirror is now the same rule). A new Completed Phases entry records the seven items of this branch, the test count, the assumptions the backend must confirm, and the eight files that fail in this environment independently of the branch. - vitest.config.ts: `.worktrees/**` joins the exclude list — a stale worktree under the main checkout is otherwise swept by `vitest run`. - connection-detail.tsx: the extra-parameters block is a fieldset with a legend and an aria-describedby hint, instead of a <label> that named nothing. - linked-accounts-panel.tsx: the token expiry is a visible, quieter second line under the status, not only a hover title a keyboard or touch user never reaches. The card's role="button" house pattern is left alone. Tests: linked-accounts-panel.test.tsx ("shows the token expiry as visible text, not only in a hover title", "omits the expiry line when the provider issued no expiry"). * fix(connections): stop refusing scope, code and grant_type as reserved OAuth parameters The mirror's reserved-name list carried three names the backend's RESERVED_OAUTH_PARAM_NAMES does not — scope, code and grant_type — so an extraAuthParams entry the save would have accepted was blocked before it left the browser, which is the one direction a mirror must never err in. The list is now exactly the backend's eight, and the paramReserved message no longer names scope as one of them in any locale. Regression: isReservedOAuthParamName "is no wider than the backend's list". * fix(connections): judge an extra parameter value by the backend's credential prefixes The mirror refused any extraAuthParams value that was one 32+ run of the token alphabet and accepted everything else. The backend has never done that: its CREDENTIAL_SHAPED_VALUE is a prefix list (sk-, xox?-, gh?_, github_pat_, AKIA, eyJ, Bearer/Basic followed by whitespace). So a UUID audience or an opaque 40-character tenant id was blocked before it left the browser while the backend would have saved it, and sk-live-x sailed through to a 400. The check is now the backend's regex, applied to the raw value as the backend applies it. HANDOFF.md's "assumptions the backend must confirm" paragraph recorded both wrong rules; corrected. Regression: validateParamValue "the backend's prefix list, exactly" and "accepts a long opaque value the backend accepts". * fix(connections): trim the name on the way out instead of trimming it before validating The backend matches a connection name against its grammar untrimmed and refuses surrounding whitespace ("Not trimmed" in its own source), while the mirror trimmed before testing — so " jira" passed every client-side check and the wizard sent it to be refused with a 400 about a character the author could not see. toStoredConnection now trims the name (a no-op on any stored document, since none carries whitespace) and validateConnection judges the raw name exactly as the backend does. Regression: validateConnection "refuses surrounding whitespace, as the backend does", toStoredConnection "trims the name", and the wizard test "sends the name without the whitespace around it". * fix(connections): restore the stored binding and proxy-trust flag when the type returns changeAuthType derived the binding from the draft alone. A caller-supplied connection mis-clicked to BASIC is corrected to SERVICE there (BASIC has no other binding), and SERVICE is also legal for STATIC — so switching back, which is a return to the stored type and needs no confirmation, arrived as a shared-key connection with no key, and the save sent a binding the user never chose. allowUnverifiedPrincipal was lost the same way on a detour through STATIC and back: forced off on the way out and never restored, so one mis-click tightened a saved proxy-trust posture on the next save. Returning to the stored type now restores both from the stored document; every other type still derives them from the draft. Regression: "keeps a caller-supplied binding across a type mis-click and back" and "restores a stored proxy-trust flag when the type returns to a user login" — both fail against the previous logic. * fix(connections): keep a half-typed connection reference editable until it is complete The picker swapped its input for a Connection chip as soon as the value started with `${connection:`, a prefix check, so the input unmounted before the name or the closing brace could be typed. The chip now renders only when parseConnectionReference returns a complete, valid reference. An unfinished or malformed one stays an unmasked text input, and is kept out of the vault chip, which the shared reference-prefix test would otherwise have sent it to. Vault-reference behaviour is unchanged. * fix(connections): retry only network failures and 5xx on the connection queries useMyConnections and useConnectionDescriptors retried every failure except 401/403/404, so a definitive 400 or 409 was replayed, once and twice respectively, delaying the error the user needs to read. Both now share an allow-list predicate: a network failure (ApiClient's status 0) or an HTTP 5xx is retried with the existing count; every other failure, including an error that is not an API error at all, is not. * fix(connections): hold a connection reference to the backend's name grammar CONNECTION_CANONICAL accepted any brace-free body up to 256 characters, so `${connection:bad name}` parsed as a valid reference and rendered as a chip for a name no connection can be saved under. The grammar `^[A-Za-z0-9][A-Za-z0-9._-]{0,63}$` now lives once in connection-name.ts and both readers import it: connection-validation.ts for the name a connection is saved under, and secret-reference.ts for parsing a reference and for building one. toConnectionReference returns null for a name outside the grammar, and the reference picker no longer offers such a connection for insertion. A malformed reference stays an editable input and reaches the existing wrapped-reference warning instead of the chip. * fix(connections): compose the connection picker's Retry action from the Button primitive The Retry action in the picker's failed state was a hand-rolled button. It now uses the Button primitive (link variant, sized to sit inline in the sentence) and carries a test id, and a test proves it refetches the list. The trigger, the listbox option rows and the inline hints stay as they are: the trigger is an input-group addon rendered beside the vault opener, which has identical hand-rolled styling, and the shared EmptyState and ErrorState are page-scale components that would overflow the popup. * feat(models): suggest Claude Fable 5.1, GPT-6 Astra and Gemini 3.8 Flash, 3.7 Flash and 3.5 Pro Adds the new model identifiers to the shared autocompletion catalogue used by the agent wizard and the Platform Operator activation form: claude-fable-5-1 (Anthropic), gpt-6-astra (OpenAI), and gemini-3.8-flash, gemini-3.7-flash and gemini-3.5-pro (Gemini and Vertex AI). claude-opus-5, claude-sonnet-5 and the gpt-5.6 Sol/Terra/Luna models were already listed. * fix(ui): give empty states, dialogs and editor panels room from their borders An audit of every Manager page and dialog, with collapsed sections expanded, measured the gap between form fields or text and the nearest bordered container, and fixed every spot under 12px: - EmptyState and ErrorState had no horizontal padding, so long text ran to the dashed border. - The create-connection and share dialogs rendered their content without the p-5 body every other dialog uses. - AlertDialog was full width with no gutter on narrow screens. - Bordered sub-panels holding fields or text used p-2/p-2.5: instruction rows in the API-calls, MCP-calls and LLM editors, LLM notes, the task RAG panel, the output alternative-group box, the RAG chunking preview, and the group config tiles, phase card, negotiation proposals, artifact content and group editors. All are now p-3. * fix(models): drop gemini-3.5-pro, which Google has not released Google announced Gemini 3.5 Pro at I/O in May 2026 but has not shipped it, and its developer documentation lists no gemini-3.5-pro model. The autocompletion catalogue should only offer identifiers a provider actually serves. * test(connections): wait for the refetched connection list, not just the request The Retry test asserted the failed state was gone as soon as the second request reached the mock, before React Query had rendered the result, so it could fail intermittently. It now falls through to the default connection mock on retry, waits for the failed state to disappear and for the jira option to render.
…rface and at every width (#209) * fix(groups): one reader for group messages and decisions, on every surface and at every width Verdicts and other group-discussion output still rendered as raw JSON after #206. Each earlier fix had covered some of the seven places that show group content — the Manager transcript, the Workforce board, the history viewer, the 1:1 chat, the "Ask more" card, the verdict card and the markdown export — and several shapes EDDI writes had never been handled anywhere. One reader, every surface: - lib/group-entry-body.ts reads a message body once: a contract card, a task or verification list, a failure notice, an abstention, or markdown. Every surface renders from it, and the export and clipboard serialize from it. - lib/group-decision.ts reads a DecisionRecord: a debate's headline apart from its reasoning, an arbitration's ruling without a false "Tie", a vote's totals and turnout without its ballot records, an agreement's terms and concessions without proposal and agent ids. Messages: the chat renders a member agent's verdicts, ballots, bids, bargains, retros and plans; a bargaining move keeps the reasoning after its JSON; LLM plans and legacy verification sheets are lists on the board and history viewer; the debate synthesis no longer shows "— ### Verdict"; the failure placeholder is a notice; a human member's literal PASS reads as an abstention; planned tasks name their assignee; the bid rationale is visible. Crashes exposed by live data: a CONVERGENCE entry has no speaker id, and hashColor(undefined) took every Delphi-style transcript down; the ops-task-force template stores dynamicAgents without provider or model lists, and the config panel took the group page down. Layout: the task board switches to its kanban on its own width, not the viewport's (columns were ~75px on a tablet and a 1440px desktop); the group header, the phase-flow breadcrumb and the vote tally wrap; a long pinned question is clamped; a history deep link opens its conversation on a phone; readable text under 10px is 10px. Tests: group-rendering-matrix renders every contract and decision shape through every surface; readers, decisions and each live-found regression have their own tests. Verified live against a running EDDI across ten discussion variations on the Manager group page, the Workforce board and the history viewer at 375px and 1440px. * fix(groups): address review on the group rendering pass - Use the shared Button (link variant, compact) for the verdict card's body toggle, the plan item "Show more" and the pinned question toggle, instead of hand-rolled buttons. - Name a live stream's planned assignees from the group roster: the Manager page passes conversation={null} while streaming and the Workforce board has no conversation yet, so both fell back to agent ids. - Localize the plan item priority prefix (groups.priorityShort, 11 locales). - Pass the group's configured tasks to the Workforce board and history viewer, so a pre-configured plan's one-line summary expands into its tasks there too. - Tests: look decision fixtures up by name, tie the markdown export assertion to the exported decision, and pin the board's configured-plan expansion and the roster name fallback.
…nd 6.4.0 (#210) * fix(approvals): a readable diff when the proposed body is not JSON, and 6.4.0 The operator's updateLlm call proposed an LLM config with one closing brace too many. It parsed on neither side of the approval, so the stored document was pretty-printed and key-sorted while the proposed one stayed a single raw line: every stored line removed, one line added, nothing to compare. - Re-indent a side that does not parse (whitespace only, never repaired), so a stray bracket shows as its own line. Text that does not open as an object or array is compared as written. - Against a broken side, ignore indentation in the comparison and print the parsed side in the broken side's key order, so neither mis-nesting after a stray closer nor a moved key reads as a change. - Mark the words that changed within paired lines (<ins>/<del>, bounded by a per-pair timeout and a render budget), and summarise added, removed and unchanged lines in a sticky legend. - Warn on a whole-document write that is not JSON, and tell the two outcomes apart: broken inside, EDDI rejects it; complete but followed by more text, Jackson stops at the end of the document and the write can succeed with the tail dropped. Hedged when redaction markers are present. - Scan the leading document for capability grants when text trails it. A trailing brace used to skip the scan entirely while EDDI would still store the grant. - Bump the Manager to 6.4.0. * fix(approvals): keep text after a closed container apart from its bracket reindentJsonText treated a closing } or ] as punctuation, so trailing text after a complete document ({"a":1} true) printed glued onto the bracket — the very case the approval's trailing-text warning is about. A closed container now counts as a finished value.
…nd ui/chat Post-import fixups for the two histories merged under ui/: - move the Manager's helper scripts from .github/scripts to scripts/ and fix the two ROOT computations that assumed the old depth (check-i18n, refresh-openapi-operations) plus the two tests that import them by path - drop the Manager's .github (CI moves to the root ci.yml), husky/lint-staged, the deploy-to-local scripts and renovate.json - untrack ui/chat/dist and ui/manager/.claude/settings.local.json - retarget the Chat build from ../EDDI/src/main/resources to dist/ - prune husky/lint-staged from the Manager lockfile by hand: npm on Windows also drops the nested @emnapi entries Linux npm ci needs
…mage, publish that image Phases 2-5 of planning/monorepo-migration-plan.md, as one commit: deleting the committed bundles without the Maven wiring would leave a commit whose jar serves a blank /manage. - manage/welcome/workforce.html move into ui/manager as Vite multi-page inputs sharing one hashed bundle; the Chat builds to dist/. frontend-maven-plugin runs npm ci + npm run build for both (Node 20.20.2 in ui/node) and copy-ui-bundles copies both dist/ trees into the jar, without the MSW worker and without overwriting index.html, robots.txt or landing-redirect.js. -DskipUi=true skips it all. 756 generated files are no longer tracked. - .gitignore, the default-resource excludes and the maven-clean-plugin fileset list the same 13 paths, so a stale pre-migration bundle can neither be committed, copied into the jar, nor survive ./mvnw clean. - Resource tests read stand-in shells from src/test/resources; StaticAssetCachingTest checks the hashes of the assets actually built. - ci.yml: UI Build & Test, UI Gate, Build Image (built once, shared as an artifact, packaged asset set checked), Backend E2E on the image from this commit (shipped-shell check, blocking OpenAPI snapshot check), E2E Gate, CodeQL Analysis (UI). docker publishes the tested image instead of building one; preflight-check certifies it. Maven jobs that ship no UI pass -DskipUi=true. - ui/manager/src/test/mocks/openapi-operations.json refreshed against the image built from this commit (+8 operations, -2 now-hidden /chat shells), which the new blocking snapshot check requires; the five EXEMPT entries in openapi-contract.test.ts that it made stale are removed. - e2e/fullstack/resources-crud.fullstack.spec.ts: the card test still clicked the pre-v6 `resource-type-behavior` id the UI no longer renders. Hidden while the Manager's full-stack tier could not boot a backend; running it against the image built from this commit exposed it. Its known-failure header is updated: with EDDI 60188c2 in the image, all six resource types list correctly. - Chat react-router 7.13.1 -> 7.18.3 (high advisories); Manager compose files take EDDI_IMAGE and the two HighValueSurfaceGuard opt-outs; Dependabot npm entries replace Renovate; scheduled CodeQL covers TypeScript; one Node pin in mise.toml; docs describe the new layout; changelog entry.
…ackage time
Findings from two independent efficiency reviews of the monorepo wiring.
- pom.xml: the five frontend-maven-plugin executions and copy-ui-bundles move
from generate-resources/process-resources to prepare-package. compile, test
and quarkus:dev no longer reinstall node_modules and rebuild both UIs (about
two minutes each, and it deleted node_modules under a running npm run dev);
package, verify and install still always build them. Defaulting skipUi to
true was rejected: a local package would silently build an image without UI.
- ci.yml:
- a `backend` filter (`code` minus ui/**, plus ui/**/*.md); on pull requests
Build & Test and Integration Tests gate on it, on push and tags on `code`,
so UI-only and npm Dependabot PRs skip the Java suite while nothing
publishes untested. BuildQualityGatesTest keeps the two lists in step.
- pom.xml and mise.toml leave the `ui` filter; the UI jobs never run Maven.
- UI Build & Test becomes UI Manager Checks, UI Manager E2E (MSW) and UI Chat,
in parallel, all required by UI Gate. The production-build checks move into
Build Image, which already builds both UIs, now with an npm cache.
- the OpenAPI snapshot check runs on the MongoDB leg only and uploads the
regenerated file when it fails.
- StaticAssetCachingTest drops its built-asset test (the UI is no longer built
before `test`); Build Image checks the hashes where the assets are produced.
- Docs, mise.toml and the changelog follow; the changelog also withdraws the
advice to enable GitHub's CodeQL default setup, which is not configured and
would reject this workflow's uploads.
- e2e-fullstack: the integration and full-stack Playwright tiers drive the bundle EDDI serves (PORT=7070, E2E_AGAINST_BACKEND=1 skips the Vite dev server in playwright.config.ts), so they test what ships. - OpenAPI Snapshot job: Build Image stores the OpenAPI document (quarkus.smallrye-openapi.store-schema-directory) and the job checks the Manager's snapshot against it without a container; refresh-openapi- operations.mjs reads OPENAPI_FILE. Required by E2E Gate and docker; the runtime check stays as a parity guard. - MSW Playwright tier runs with two workers in CI (234 tests: 5.4 and 5.8 min, no flakes, against 13.1 min). Vitest css:false rejected: slower, and it broke a test. - Dependabot npm entries: cooldown and a security-updates group, validated against the published schema. - README.md, AGENTS.md and .githooks/** move from the `code` filter to `backend`: a change to only those runs Build & Test and no image chain. - Integration Tests restore Build & Test's jacoco.exec and surefire reports and pass -DskipUTs=true instead of running ~20k unit tests again; skipUTs follows skipTests, and BuildQualityGatesTest guards the hand-off.
Contributor
|
Important Review skippedToo many files! This PR contains 1350 files, which is 1250 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. Usage-priced reviews support at most 300 files. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (489)
📒 Files selected for processing (1350)
You can disable this status message by setting the |
Dependency Review SummaryThe full dependency review summary was too large to display here (1821KB, limit is 1024KB). Please download the artifact named "dependency-review-summary" to view the complete report. |
Contributor
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
- ui/manager/.ds-sync/package-lock.json: brace-expansion 5.0.7 -> 5.0.12 (GHSA-mh99-v99m-4gvg, GHSA-rgw5-rvv9-x895, both high), the one entry that failed Dependency Review and Trivy. The nested @emnapi entries npm on Windows prunes are kept. The directory gets its own Dependabot entry. - .github/codeql/codeql-ui.yml scopes the TypeScript analysis (ci.yml and the scheduled codeql.yml) to ui/manager/src and ui/chat/src, the code that ships; without `paths` it also graded MSW's generated worker, a dev script and the Helm chart's YAML. .github/codeql/** joins the code and backend filters. - use-current-screen-context.ts: toContextPayload accepts only the known context keys and builds the payload with Object.fromEntries, with a test that unknown keys are dropped (the CodeQL property-injection alert). - Changelog records the three findings and their fixes.
There was a problem hiding this comment.
🟡 Changes recommended
CI gating, stale skipUi output, path-filter coverage, and frontend SBOM completeness remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Implements #670 by consolidating both frontend repositories into EDDI and integrating their builds with Maven and CI.
Changes:
- Imports Manager and Chat UI source histories under
ui/. - Builds frontend assets during Maven packaging instead of committing generated bundles.
- Reworks CI to test and publish the same container image.
File summaries
| File | Description |
|---|---|
pom.xml |
Adds frontend build and resource-copy lifecycle. |
.github/workflows/* |
Adds UI checks, image-based E2E, CodeQL, and deployment gates. |
.github/dependabot.yml |
Adds npm dependency management. |
ui/manager/** |
Imports Manager application, tests, and tooling. |
ui/chat/** |
Imports Chat UI application, tests, and tooling. |
src/main/resources/** |
Removes committed generated frontend assets. |
src/test/** |
Adds shell stand-ins and updates resource tests. |
| Root config/docs | Updates ignores, toolchain, and monorepo documentation. |
Review details
- Files reviewed: 103/1834 changed files
- Comments generated: 4
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- pom.xml: -DskipUi=true could still ship a UI. Skipping copy-ui-bundles cannot un-copy, so `package` followed by `package -DskipUi=true` in the same target/ packaged the first run's bundles. maven-clean-plugin gains a drop-stale-ui-bundles execution on prepare-package that removes the thirteen generated paths from target/classes and leaves the three backend-owned files. It runs unconditionally: Vite content-hashes its filenames, so a repackage without a clean also shipped the previous bundle beside the new one. The plugin moved above maven-resources-plugin (same-phase executions run in POM order) and both filesets moved onto their executions, so the clean-phase list cannot reach the new one and delete ui/manager/dist mid-build. - ci.yml: Preflight Dry-Run (PR) listed build-and-test in `needs` although it consumes only build-image's artifact. build-and-test gates on `backend`, so a UI-only PR skipped it and GitHub skipped preflight before evaluating its own `code` condition — the image that PR would publish went uncertified. BuildQualityGatesTest now grades that shape for every job. - ci.yml: .github/dependabot.yml joins the `backend` filter. BuildQualityGatesTest reads it to check the Docker ecosystems stay in step with base-image-check.yml; Dependabot's own check grades the schema, not that contract. - ci.yml: the SBOM job also emits CycloneDX BOMs for both UIs (@cyclonedx/cyclonedx-npm, --package-lock-only --omit dev). They build into the jar, so their npm production dependencies are part of the shipped supply chain and the Maven-only BOM described just over half of it. - docs: the CodeQL and CycloneDX rows in security.md, and the SBOM section of build-reproducibility.md, now say what those two actually cover.
…und it Quarkus OIDC reads roles from the `groups` claim whenever that claim is present and never falls back to realm_access. EDDI already owns `groups` — workspaces resolve `team:<group>` spaces from it — and the realm this project ships puts the seeded `eddi` administrator in the `engineering` group. So installing the chart with keycloak.enabled=true and giving `eddi` a password as NOTES.txt instructs produced an administrator who authenticated and was then refused every endpoint, /administration/* included. Group-less users behaved correctly, which is why nothing showed it. Fixed with quarkus.oidc.roles.role-claim-path. Pre-existing, not from the monorepo migration — but nothing had ever executed an authenticated request, so: - ui/manager/docker-compose.integration-keycloak.yml boots EDDI with OIDC ENFORCED beside Keycloak 26. Every other backend compose file sets EDDI_SECURITY_ALLOW_UNAUTHENTICATED, which makes @RolesAllowed a no-op. KC_HOSTNAME pins the issuer so tokens fetched through the published port are accepted by the backend validating on the compose network. - A Playwright `auth` project (7 tests) and a CI job, Auth E2E (Keycloak), folded into the E2E Gate so a red run blocks publishing. - scripts/make-test-realm.mjs derives the tier's realm from helm/eddi/files/eddi-realm.json and adds the throwaway admin password that realm deliberately omits. Generated, never committed, and every assumption it makes is asserted. - Deleted ui/manager/keycloak/eddi-realm.json: a committed copy that had drifted until its client id (eddi-manager) and roles (admin/editor/viewer) matched nothing the backend serves or enforces, so it could not have logged anyone in. Both Keycloak compose files now mount the chart's realm. - playwright: fullyParallel off at the top level, on for the mock-backed `ui` tier. The backend-facing tiers share one EDDI, so running their files at once raced — CI hid it behind workers: 1. Verified: 7/7 with the fix and exactly one failure (the administrator) without it; 79/79 on the unauthenticated MongoDB tiers with the same image; and 44 + 35 on PostgreSQL, which had never run on this branch.
… on build-image DeploymentManifestsTest still required preflight-check to need build-and-test, which failed Build & Test once 44fb8ff dropped that dependency. The ordering it enforced (never publish for a commit whose tests failed) applies to sbom, which uploads; preflight-check is a pull-request dry run that pushes only to a registry inside the job. For it the test now requires build-image, the artifact it certifies, and forbids build-and-test, whose backend gate skips it on UI-only PRs.
notify-slack classified a run as "Build Passed" unless one of the job results it reads was a failure, and e2e-auth was not among them — so a run whose only red job was the new auth tier would have been announced as green. It now needs e2e-auth and folds its result into the Backend E2E line.
rolandpickl
approved these changes
Sep 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
⚠ Merge with "Create a merge commit" — never squash. This PR imports the full histories of
labsai/EDDI-Manager(1,213 commits) andlabsai/EDDI-Chat-UI(62 commits). A squash-merge flattensthem into one commit and destroys the path-scoped history (
git log -- ui/manager/…,--follow,blame) the import exists to keep.
What this does
Executes
planning/monorepo-migration-plan.md(#670): the Manager and the Chat UI move into thisrepository as
ui/managerandui/chat, Maven builds both into the jar, generated bundles are nolonger committed, and CI tests the image built from the PR before it is published.
2e4f3340a,628825391git filter-repo --to-subdirectory-filter+ unrelated-history merges). Trees are blob-identical to Managermain@0870ae87and Chatmaster@71fa395; no tags importedc23ff00b9.github/, dead deploy scripts/husky/renovate removed, Chat builds todist/298bde447Build Imageonce,Backend E2Eon the built image,dockerpublishes the tested bytes); backend tests, docs, changelog4f19e4767prepare-package(compile/test/dev never run npm);backendfilter so UI-only PRs skip the Java suite; UI jobs split and parallel; npm cache1c174896fOpenAPI Snapshotjob from the build-time document; MSW tier on two workers; Dependabot cooldown + security group; test-only root files out ofcode; Integration Tests reuse the unit-test coverage dataWhat to review
CodeRabbit and Copilot skip PRs this size, so this needs a human review of the mechanics. The
imported trees are the source repositories' default branches and need no line-by-line review.
pom.xml—skipUi/skipUTs, frontend-maven-plugin,copy-ui-bundles, resource excludes, clean fileset.github/workflows/ci.yml,codeql.yml,.github/dependabot.yml.gitignore,.gitattributessrc/main/resources/META-INF/resourcesandui/manager/.githubStaticAssetCachingTest,Rest*ResourceTest+ stand-in shells,BuildQualityGatesTestui/manager/{vite.config.ts,playwright.config.ts,manage.html,welcome.html,workforce.html},ui/chat/vite.config.tsdocs/changelog.md— the two newest entries record every decision, deviation and verificationVerified locally
compile/testrun no npm;clean packagebuilds both UIs inprepare-packagebefore the copy; the jar carries 737 hashed assets and no MSW workernode:20: Managernpm ci, audit, lint, i18n, typecheck, Vitest, build, 234 Playwright MSW tests; Chatnpm ci, typecheck, tests, buildmain)create-secrets.ps1cases that fail identically on untouchedmainon the same machineFirst signal here, not locally: the PostgreSQL Playwright tiers, the merged 90/80 coverage gate
with the new unit-test coverage hand-off, and the job graph itself (skips, artifacts, gates).
After merge (outside this repo)
UI GateandE2E Gate; keepCodeQL AnalysisandBuild & Testmainrun: pointer READMEs, close the old repos' open PRs (Manager Facebook implementation bugfix #72, Any plans to support PostgreSQL ? #95, Feature/httpcalls improvements #140, Update dependency io.swagger:swagger-annotations to v1.6.2 - autoclosed #168, Update dependency org.infinispan:infinispan-core to v9.4.19.Final #207; Chat Deploy EDDI to server #19–enabled all other apis on swagger ui #24, Issues/eddi 2 #26), archive both reposfix/release-6.2-polish(two commits never merged tomaster)