Conversation
zielivia
added a commit
that referenced
this pull request
Apr 29, 2026
CI shard 9 trace (run 25095248488 retry #1): - safeFill toHaveValue 60s timeout, 59 polls = textarea empty under parallel-shard load. - Initial run safeFill passed but waitForResponse 180s timeout, page snapshot showed the inline reply textarea empty at 180s — controlled state had been silently dropped between safeFill (typed body) and the submit button click. keyboard.type races the React state commit when MessageComposer mounts inside MessageDetailPageClient and runs its own effects in parallel — characters land but are immediately overwritten before the caller can proceed. locator.fill is atomic (`element.value = …` + dispatched input event) and removes the per-keystroke race entirely. Two fixes: 1. safeFill switched from click + focus + Ctrl+A + Delete + keyboard.type to locator.fill, with toHaveValue staying as the commit gate. 2. Pre-submit re-assertion in the inline reply scenario — fail loudly on a state drop instead of silently sending an empty-body POST that the response filter rejects. Local: TC-MSG-009 passes in 6.3s.
zielivia
added a commit
that referenced
this pull request
Apr 30, 2026
CI shard 9 trace (run 25095248488 retry #1): - safeFill toHaveValue 60s timeout, 59 polls = textarea empty under parallel-shard load. - Initial run safeFill passed but waitForResponse 180s timeout, page snapshot showed the inline reply textarea empty at 180s — controlled state had been silently dropped between safeFill (typed body) and the submit button click. keyboard.type races the React state commit when MessageComposer mounts inside MessageDetailPageClient and runs its own effects in parallel — characters land but are immediately overwritten before the caller can proceed. locator.fill is atomic (`element.value = …` + dispatched input event) and removes the per-keystroke race entirely. Two fixes: 1. safeFill switched from click + focus + Ctrl+A + Delete + keyboard.type to locator.fill, with toHaveValue staying as the commit gate. 2. Pre-submit re-assertion in the inline reply scenario — fail loudly on a state drop instead of silently sending an empty-body POST that the response filter rejects. Local: TC-MSG-009 passes in 6.3s.
zielivia
added a commit
that referenced
this pull request
Apr 30, 2026
CI shard 9 trace (run 25095248488 retry #1): - safeFill toHaveValue 60s timeout, 59 polls = textarea empty under parallel-shard load. - Initial run safeFill passed but waitForResponse 180s timeout, page snapshot showed the inline reply textarea empty at 180s — controlled state had been silently dropped between safeFill (typed body) and the submit button click. keyboard.type races the React state commit when MessageComposer mounts inside MessageDetailPageClient and runs its own effects in parallel — characters land but are immediately overwritten before the caller can proceed. locator.fill is atomic (`element.value = …` + dispatched input event) and removes the per-keystroke race entirely. Two fixes: 1. safeFill switched from click + focus + Ctrl+A + Delete + keyboard.type to locator.fill, with toHaveValue staying as the commit gate. 2. Pre-submit re-assertion in the inline reply scenario — fail loudly on a state drop instead of silently sending an empty-body POST that the response filter rejects. Local: TC-MSG-009 passes in 6.3s.
zielivia
added a commit
that referenced
this pull request
May 2, 2026
CI shard 9 trace (run 25095248488 retry #1): - safeFill toHaveValue 60s timeout, 59 polls = textarea empty under parallel-shard load. - Initial run safeFill passed but waitForResponse 180s timeout, page snapshot showed the inline reply textarea empty at 180s — controlled state had been silently dropped between safeFill (typed body) and the submit button click. keyboard.type races the React state commit when MessageComposer mounts inside MessageDetailPageClient and runs its own effects in parallel — characters land but are immediately overwritten before the caller can proceed. locator.fill is atomic (`element.value = …` + dispatched input event) and removes the per-keystroke race entirely. Two fixes: 1. safeFill switched from click + focus + Ctrl+A + Delete + keyboard.type to locator.fill, with toHaveValue staying as the commit gate. 2. Pre-submit re-assertion in the inline reply scenario — fail loudly on a state drop instead of silently sending an empty-body POST that the response filter rejects. Local: TC-MSG-009 passes in 6.3s.
zielivia
added a commit
that referenced
this pull request
May 2, 2026
Resolves the High and Medium findings from the @patrykk-com review on PR open-mercato#1730. High: - Migration-snapshot drift on sidebar_variants: the snapshot still listed the legacy `sidebar_variants_user_id_tenant_id_locale_name_unique` constraint even though Migration20260427124900 + 20260427143311 dropped it and replaced it with a partial unique index `WHERE deleted_at IS NULL` (which a `@Unique` decorator cannot represent). Drop the @unique decorator on `SidebarVariant` and remove the stale snapshot entry; partial index is owned by raw SQL in the migration. A follow-up `yarn db:generate` now diffs cleanly. (H #1) - Move inline zod schemas (sidebarSettingsSchema, createVariantInputSchema, updateVariantInputSchema, variantRecordSchema) from variants route handlers into `data/validators.ts` and import them in both routes. Settings shape is shared with `sidebarPreferencesInputSchema` so the constraint definitions no longer drift. (H #2) Medium: - Replace `as any` / `: any` across the new sidebar code with `EntityManager` + typed `FilterQuery`. `parsed.data.settings as any` casts are gone now that service signatures accept `Partial<SidebarPreferencesSettings>` (which matches the inferred zod type). (M #3) - Add explicit one-line rationale on every empty-catch block in AppShell (localStorage / cookie blocked in private mode — non-critical) and SidebarCustomizationEditor (`window.dispatchEvent` with no listener — AppShell refreshes on next navigation). (M #4) - Replace raw `<button>` drag handle in SortableItemRow with `<IconButton variant="ghost" size="sm">` and use the existing forwardRef so `setActivatorNodeRef` and dnd-kit listeners still wire correctly. (M open-mercato#5) - i18n hardcoded strings in SidebarPreview (`Search...`, `No groups to preview.`, `Drag to reorder`) — wrapped in `t(...)` and added 3 new keys to en/pl/de/es. (M open-mercato#6) - Switch primitive: replace inline `shadow-[0_1px_2px_rgba(10,13,20,...)` arbitrary-value shadow with the new `--shadow-switch-thumb` CSS custom property in light + dark themes (and synced into the standalone template globals.css). Switch now uses `shadow-switch-thumb` Tailwind utility. (M open-mercato#7) - Behavior regression for non-admin users: `requireFeatures: ['auth.sidebar.manage']` on the sidebar-customization page meta locked every non-admin user out of personal-scope customization, even though the variants/preferences APIs only gate role-application via that feature. Drop the page-level requireFeatures so any authenticated user can reach the page; the editor already conditionally hides "Apply to roles" via `canApplyToRoles` (server-checked against `auth.sidebar.manage`). (M open-mercato#8) New tests: - 6 unit tests in `sidebarPreferencesService.scope.test.ts` lock down the cross-tenant + cross-user scope guards on `loadSidebarVariant`, `updateSidebarVariant`, `deleteSidebarVariant`. Each test stubs `findOneWithDecryption` and asserts the exact `{ id, user, tenantId, deletedAt: null }` filter shape so a future refactor can't silently drop the user or tenant filter. (M open-mercato#9) All 405 core test suites (3,329 tests) and 71 UI test suites (363 tests) pass; build:packages clean across 18 packages.
pkarw
pushed a commit
that referenced
this pull request
Jun 17, 2026
… (report-high #1) Fixes finding #1 from report-high.md: cross-user OpenCode session continuation enables privilege escalation. Root cause: handleOpenCodeMessage / handleOpenCodeMessageStreaming / handleOpenCodeAnswer accepted a caller-supplied sessionId and resumed the matching OpenCode session without verifying ownership; the chat route only minted a fresh session-token api_key when !sessionId, so any authenticated user with ai_assistant.view could resume another user's OpenCode session and execute MCP tools under that identity. Fix: add additive api_keys.opencode_session_id column with a partial unique index; bind it on the first 'done' event in the chat route via new bindOpencodeSessionToApiKey helper; assert (sessionUserId, tenantId, organizationId) ownership via new findApiKeyByOpencodeSessionId on every resume; surface opaque 'Session not available' on all failure paths to prevent enumeration. Replace unscoped getPendingQuestions() with owner-scoped getOwnedPendingQuestions(em, auth); the deprecated overload now throws to fail loudly. Regression: api_keys.opencodeBinding.test.ts (5 cases), opencode-handler-ownership.test.ts (10 cases), chat-route-ownership.test.ts (8 cases), TC-AI-CHAT-OWNERSHIP-001-opencode-session.spec.ts (4 cases). Spec: .ai/specs/2026-05-24-fix-opencode-session-ownership.md. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
pkarw
pushed a commit
that referenced
this pull request
Jun 17, 2026
Sibling to dba38484d. Two commits because amending the original would rewrite its SHA in turn — the tracker now matches HEAD exactly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
pkarw
pushed a commit
that referenced
this pull request
Jun 17, 2026
) Fixes finding #3 from report-high.md: cross-tenant role create/update/delete via body-supplied tenantId. Root cause: roles route wired POST/PUT/DELETE through makeCrudRoute with rawBodySchema=z.object({}).passthrough() and mapInput=parsed, forwarding body.tenantId unchanged to commands whose lookups used a null-scoped filter ({tenantId: null, organizationId: null}). Any tenant admin holding auth.roles.manage (granted via the seed admin role's auth.* wildcard) could create roles in foreign tenants, reassign/rename roles in any tenant, or delete roles in any tenant. Fix: route-layer guard via enforceRoleTenantAccess (already existed for create/update; extended with a 'delete' mode here) wired into every mapInput; command-layer defense-in-depth via resolveActorScope + buildScopedRoleFilter so update/delete lookups are tenant-scoped for non-superadmins (404 on cross-tenant) and create+update reject body.tenantId that differs from auth.tenantId for non-superadmins (403). Migrated the touched em.findOne to findOneWithDecryption. Regression: - packages/core/src/modules/auth/lib/__tests__/roleTenantGuard.test.ts (7 new delete-mode cases) - packages/core/src/modules/auth/commands/__tests__/roles.tenant-move.test.ts (updated tenant-move contract + new create/delete tenant-scoping cases) - packages/core/src/modules/auth/api/__tests__/roles.route.test.ts (8 new wiring tests asserting cross-tenant rejection on each mapInput) Local typecheck/jest blocked by pre-existing TS5103 (jest ignoreDeprecations: '6.0' vs TS 5.9.3) — same blocker as #1 and #2; build:packages, generate, i18n:check-sync all pass; CI must validate the typecheck+test legs. Deferred Low follow-ups: L1 replace rawBodySchema passthrough with strict schemas whitelisting tenantId; L2 align delete cross-tenant response to 404 to match command-layer 404. Sibling pattern flagged for separate tracker entry: packages/core/src/modules/auth/api/users/route.ts uses the same passthrough + passthrough mapInput pattern and likely has an analogous cross-tenant User.tenantId vulnerability. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
pkarw
pushed a commit
that referenced
this pull request
Jun 17, 2026
…sers (tracker #4) Fixes finding #4 from report-high.md: Hardcoded default password 'secret' for derived admin/employee users. Root cause: setupInitialTenant fell back to the literal 'secret' for OM_INIT_ADMIN_PASSWORD / OM_INIT_EMPLOYEE_PASSWORD when the env vars were unset, and 'mercato auth setup' hardcoded includeDerivedUsers: true while only surfacing the primary user's password in stdout — leaving silently-seeded admin@<domain> and employee@<domain> accounts with the publicly known default in any production deploy that didn't pre-set the env vars. Fix: - setup-app.ts: remove the 'secret' literal; generate 16-char base64url passwords via randomBytes(12) when no env override is supplied; surface them on users[].generatedPassword. - setup-app.ts: new DerivedUserPasswordRequiredError + production safeguard that throws when includeDerivedUsers is true, allowDemoDerivedPasswords is unset, and NODE_ENV=production lacks the env overrides. No partial DB state. - setup-app.ts: additive SetupInitialTenantOptions.allowDemoDerivedPasswords?: boolean and SetupInitialTenantResult.users[].generatedPassword?: string | null (BC-safe). - auth/cli.ts: new --include-demo-users flag (default off) controlling derived seeding; output loop now prints each generated password with a 'GENERATED — copy now' warning; catches DerivedUserPasswordRequiredError → exit 2. - cli/init-secrets.ts: drop DEFAULT_DERIVED_PASSWORD = 'secret'; randomize unconditionally when overrides are unset; OM_INIT_GENERATE_RANDOM_PASSWORD becomes a deprecated no-op with a one-time warning. - cli/mercato.ts: 'mercato init' passes --include-demo-users so the dev/demo bootstrap keeps seeding admin@/employee@ behind the same opt-in contract. Regression: - packages/core/src/modules/auth/__tests__/cli-setup-demo-users.test.ts — 4 cases: default no-seed; opt-in with autogenerated; opt-in with env overrides; production safeguard throws DerivedUserPasswordRequiredError. - packages/cli/src/lib/__tests__/init-secrets.test.ts — rewritten to assert no 'secret' default, base64url randomization, and that OM_INIT_GENERATE_RANDOM_PASSWORD is a no-op. Local test runner blocked by pre-existing TS5103 (jest ignoreDeprecations: '6.0' vs TS 5.9.3) — same blocker as #1/#2/#3; yarn typecheck blocked by pre-existing missing-re2js error in packages/queue. build:packages + generate + i18n:check-sync + i18n:check-usage pass. Code-review: 0 Medium+, 2 Low (deferred — dynamic import in new test; RELEASE_NOTES.md entry for the CLI default flip). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
pkarw
pushed a commit
that referenced
this pull request
Jun 17, 2026
…open-mercato#8) Fixes finding open-mercato#8 from report-high.md: MfaAdminService.resetUserMfa allowed any tenant admin (granted security.* via per-tenant admin role) to reset MFA for users in any other tenant because findUserById had no tenant filter and no caller scope was forwarded. Root cause: resetUserMfa(adminId, userId, reason) loaded the target user via em.findOne(User, { id: userId, deletedAt: null }) with no tenant scoping and the command never forwarded ctx.auth.tenantId. Fix: Add MfaAdminAuthScope type. Add overloaded resetUserMfa with scope parameter that loads the target via findOneWithDecryption, filtered by scope.tenantId for non-superadmins, with an additional cross-org check; cross-scope targets return null and surface as a unified 404 MfaAdminServiceError ('User not found') to prevent existence enumeration. Preserve the 3-arg overload as @deprecated; it now fails closed (treated as non-superadmin with tenantId=null, rejecting every load as 404). Command builds the scope from ctx.auth.tenantId/orgId/isSuperAdmin and forwards it. Regression: - MfaAdminService.test.ts (5 new cases): same-tenant non-superadmin succeeds, cross-tenant non-superadmin rejected 404 with no method soft-deletes / no recovery-code invalidation / no security.mfa.reset event emitted, superadmin override succeeds, cross-organization non-superadmin rejected 404, deprecated no-scope call fails closed as 404. - mfa-reset.route.test.ts (1 new case): MfaAdminServiceError statusCode 404 from the command bus surfaces as HTTP 404 at the route boundary. Local typecheck/test/build:app blocked by the same pre-existing 're2js' module-not-found + TS5103 jest --ignoreDeprecations issues documented in tracker #1-open-mercato#7; build:packages + generate + i18n:check-sync pass; targeted MfaAdminService + route tests pass 12/12 via root yarn jest. CI must validate the typecheck + test + build:app legs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
pkarw
pushed a commit
that referenced
this pull request
Jun 17, 2026
…pen-mercato#13) Fixes finding open-mercato#13 from report-high.md: cross-account MFA verification — challenge not bound to authenticated user. Root cause: MfaVerificationService.getValidChallenge looked up the MfaChallenge by id alone, so prepareChallenge / verifyChallenge trusted challenge.userId; the verify route then minted an mfa_verified JWT for context.auth.sub. An attacker with A's password could complete A's MFA gate using a different account B's challengeId and B's TOTP — full MFA bypass for A. The sudo prepare route shared the same shape: it forwarded session.userId as the MFA scope without verifying the SudoSession belonged to the caller, so a known sudo sessionId let any caller drive an OTP/SMS send under the victim's enrollment. Fix: add MfaVerificationAuthScope and overloaded prepareChallenge / verifyChallenge that require an authenticated { userId } scope. getValidChallenge now filters by { id, userId } and fails closed with 404 (uniform with miss/expired to prevent enumeration). Deprecated no-scope overloads throw 404 immediately without touching the DB (no UUID-cast 500). Routes forward context.auth.sub; SudoChallengeService forwards session.userId and now also accepts options.expectedUserId mirroring its verify counterpart, with the sudo prepare route passing context.auth.sub for ownership defense-in-depth. Regression: MfaVerificationService.test.ts adds 6 cases (cross-user 404 on verify/prepare, deprecated-overload 404, same-user happy path). challenge.route.test.ts asserts the scope argument is forwarded and that route surfaces the 404. sudo challenge.route.test.ts asserts expectedUserId is forwarded. Existing tests updated to thread the scope. Drive-by: fixed two `../../lib/...` → `../lib/...` relative-path bugs in packages/ai-assistant/src/modules/ai_assistant/__tests__/chat-route-ownership.test.ts introduced by tracker #1's commit 5ab0564 and noted in open-mercato#11/open-mercato#12 deferred follow-ups, so yarn test runs ai-assistant. One pre-existing test (`chat route — streaming branch wires auth + em through to the handler`) remains failing because the streaming branch depends on a populated ai-agents registry that jest cannot resolve — separate from open-mercato#13, deferred to a follow-up tracker entry. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
pkarw
pushed a commit
that referenced
this pull request
Jun 17, 2026
… reaches the handler mock Three streaming-branch tests in chat-route-ownership.test.ts asserted that the chat route invokes `handleOpenCodeMessageStreaming` (and the post-`done` binding step) but received zero mock calls because the background IIFE in `api/chat/route.ts` stalls on `writeSSE` when nothing reads `stream.readable`. A single `setImmediate` tick was not enough to clear the un-drained TransformStream's backpressure, so the IIFE never reached the handler call before the assertion ran. Fix: replace the `setImmediate` wait with a `drainSseResponse()` helper that reads `res.body` to completion. With the stream drained, every `writeSSE` resolves, the IIFE runs through to `closeWriter()` in the `finally` block, and the mocked handler is reached on the way. Tests covered: "passes the OpenCodeAuthContext built from auth.sub/tenantId/orgId", "calls bindOpencodeSessionToApiKey on a freshly minted session", "does NOT call bindOpencodeSessionToApiKey when resuming an existing sessionId". Local: 83/83 ai-assistant test suites pass (1198/1198 tests). Previously red on this PR's CI (introduced by tracker #1's commit 5ab0564 — masked locally by the now-fixed TS5103 jest config, surfaced as the merge from develop landed the jest config fix). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
pkarw
pushed a commit
that referenced
this pull request
Jun 17, 2026
… instead of 500
Standalone integration tests TC-AI-CHAT-OWNERSHIP-001 expect 403 'Session
not available' for every answerQuestion failure mode (unknown question
id, mismatched sessionId, foreign owner). CI runs the spec without a live
OpenCode backend, so `client.getPendingQuestions()` throws ECONNREFUSED
and the previous generic catch returned `{ error: <message> }` 500.
That violated finding #1's security intent: the answerQuestion short-
circuit MUST emit a uniform opaque response so an attacker cannot
distinguish "OpenCode is down" from "ownership check failed" from
"unknown question id" — every non-success outcome on this path is a
refusal-to-authorize from the caller's perspective.
Fix: drop the OpenCodeSessionOwnershipError special case and the generic
500 fallback; every catch path now returns 403 'Session not available'.
The actual error is still captured via `console.error('[AI Chat] Answer
error:', error)` so operators can debug from server logs. Removed the
now-dead `OpenCodeSessionOwnershipError` import.
Test impact: chat-route-ownership.test.ts (unit) — 8/8 pass. TC-AI-CHAT-
OWNERSHIP-001 (Playwright integration) — the two assertions on `Expected:
403` (lines 84, 111) now match the route's behaviour without needing a
live OpenCode container.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zielivia
pushed a commit
that referenced
this pull request
Jul 29, 2026
docs(analysis): WMS Month 1 gap report (PL + EN)
zielivia
pushed a commit
that referenced
this pull request
Aug 5, 2026
* feat(telemetry): add @open-mercato/telemetry package with OTLP tracing, logs, metrics Vendor-neutral OpenTelemetry telemetry as a workspace package (facade + noop/console/otlp providers, env/init), app instrumentation, and cross-boundary trace propagation. - packages/telemetry: facade (logger, tracer, meter, reportError, redact), delegation-model TelemetryProvider with globalThis registry, backup-header propagator for GCP LB root-trace continuity. - App wiring: instrumentation.node bootstrap, dispatcher reportError + http.server.request.duration metric, serverExternalPackages, .env.example. - Queue propagation: metadata._trace carrier (local strategy) + bullmq-otel (async strategy), worker-process init. - Tests: telemetry 47, queue 56, app 112. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(telemetry): init telemetry before app graph loads so worker/scheduler jobs emit pg/undici spans OpenTelemetry's pg/undici auto-instrumentation only records spans for a module required AFTER the SDK has started. Standalone worker/scheduler processes load MikroORM's Postgres driver (@mikro-orm/postgresql -> pg) during CLI bootstrap, before runWorker() called initTelemetry() — so job handlers' DB queries produced no spans and the trace showed only the bullmq-otel add/process/complete envelope with an empty job body. - packages/cli/src/bin.ts: call initTelemetry() before dynamically importing the mercato entry, for every bootstrap-requiring command (worker, scheduler, …). No-op when telemetry is disabled. runWorker's in-process init stays as an idempotent fallback. - packages/cli: add @open-mercato/telemetry dependency. - packages/telemetry: spawned-subprocess test locking that an OTLP-backed provider instruments pg so a query emits a pg.query span (jest's module system can't exercise require-in-the-middle faithfully). - spec: S4 load-order requirement, R15, Testing section, changelog. Verified end-to-end on a live OTLP backend: unfixed worker -> process span with 0 pg children; fixed worker -> full findPendingVerification query tree nested under process <queue>. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(create-app): wire telemetry into scaffolded app template (parity with apps/mercato) The telemetry feature (commit d75042e) wired @open-mercato/telemetry into apps/mercato but left the create-app template untouched, so a freshly scaffolded app had no web-tier telemetry: setting TELEMETRY_BACKEND produced silence. Per packages/create-app/AGENTS.md, app-shell changes MUST be synced to the template. This ports the six app-side wiring points: - package.json.template: add @open-mercato/telemetry dependency and bullmq-otel optionalDependency (the @opentelemetry/* SDK arrives transitively as optionalDependencies of @open-mercato/telemetry, matching apps/mercato which also does not list them directly). - src/instrumentation.ts: initialize telemetry on the Node runtime via a conditional import (edge runtime is skipped — OTEL NodeSDK is Node-only). - src/instrumentation.node.ts (new): initTelemetry() + SIGTERM/SIGINT flush, degrading to no telemetry on init failure. - src/app/api/[...slug]/route.ts: recordHttpDuration (http.server.request.duration histogram) on completion + reportError funnel on 5xx. Kept byte-identical to apps/mercato so the template-sync test passes. - next.config.ts: externalize all @opentelemetry/* packages (serverExternalPackages) so the pg/undici auto-instrumentations patch the real drivers instead of a bundled copy. - .env.example: document the TELEMETRY_* / OTEL_EXPORTER_OTLP_* variables. The worker/scheduler telemetry fix already ships via @open-mercato/cli, which now depends on @open-mercato/telemetry — scaffolded apps get it through the {{PACKAGE_VERSION}} pin once the version is published. Validation: build:packages 22/22, telemetry 48/48, create-app 61/61 (including the template↔monorepo byte-identity dispatcher sync test). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(telemetry): align package version to 0.6.5 (monorepo lockstep) The telemetry package shipped at 0.6.3 while every other public package is at 0.6.5. scripts/check-version-alignment.sh enforces lockstep against packages/shared, so 0.6.3 is a release blocker: create-app scaffolds pin all @open-mercato deps to {{PACKAGE_VERSION}} (the create-app version, 0.6.5), so `@open-mercato/telemetry@0.6.5` had no candidate and a fresh app failed `yarn install` with "No candidates found". Surfaced by yarn test:create-app. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(create-app): add telemetry files to the template sync checklist The Template Sync Checklist listed layout/dev-script parity but not the telemetry wiring, so future changes to instrumentation.ts / the dispatcher route / next.config serverExternalPackages / .env.example / package.json.template could drift out of sync (the exact gap this branch fixed). Adds entries 9–14, including the byte-identical dispatcher rule and the version-lockstep requirement for @open-mercato/telemetry. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(telemetry): gate backend log export by level; memoize queue telemetry resolution Two review fixes: - logger: gate provider.emitLog by the configured TELEMETRY_LOG_LEVEL, so below-level records no longer ship to the OTLP backend (stdout was already gated). Controls remote log volume/cost, not just stdout. - async queue: memoize the in-flight bullmq-otel resolution as a promise instead of a boolean set before the await, so concurrent first-time callers share one result and a queue/worker pair can't be built with inconsistent telemetry wiring. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(telemetry): cover backend log-level gating and concurrent queue telemetry wiring Regression guards for the two review fixes: - logger-level: below-level records must not reach provider.emitLog (fails against the ungated export). - async.telemetry: concurrent enqueue+process must wire the SAME bullmq-otel instance into both Queue and Worker (fails against the boolean-before-await memoization, which left the worker untraced). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(telemetry): sync spec with create-app parity + review fixes The spec was accurate through the 2026-07-01 worker-DB-spans entry but lagged the last five commits on the branch. Brings it up to the current tip (f95a4dd): - create-app template parity (c98aee6/34a64112/7682c955): the six app-side wiring points ported to the scaffold template, version lockstep fix (0.6.3 -> 0.6.5), Template Sync Checklist entries, and the live SigNoz scaffold validation -- documented in touched-areas, Testing, and a changelog entry (the spec had zero create-app mention). - review fixes (5db884a): backend log export now gated by TELEMETRY_LOG_LEVEL; async-queue bullmq-otel resolution memoized as an in-flight promise -- captured in the env table, touched-areas, and a changelog entry. - new regression tests (f95a4dd): logger-level + async.telemetry; updated counts (telemetry 48 -> 50, queue 58). Spec-only change; no product code touched. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(telemetry): shrink app adoption surface via @open-mercato/telemetry/nextjs Existing apps had to hand-copy the web-tier wiring (a 15-entry serverExternalPackages list, an instrumentation.node.ts, an inline HTTP metric fn), and a *partial* externals copy silently disabled exporting — the #1 footgun. Move the boilerplate into the package so apps and the scaffold template consume helpers instead. New packages/telemetry/src/nextjs.ts subpath (resolved via the ./* export; import-safe from next.config.ts — never statically imports @opentelemetry/* or the pino logger, dynamic-imports ./init inside the register fn): - telemetryServerExternalPackages — the full @opentelemetry/* list as a single source of truth to spread into serverExternalPackages; a partial copy is now impossible. - registerTelemetryForNextjs() — one-line instrumentation.ts bootstrap owning init + graceful degrade + SIGTERM/SIGINT flush + edge-runtime skip. Deletes instrumentation.node.ts from apps/mercato and the template. - recordHttpDuration() — the semconv http.server.request.duration histogram, moved out of the inline dispatcher (route.ts stays byte-identical app<->template, now importing the helper). apps/mercato (next.config.ts / instrumentation.ts / route.ts) and their template mirrors consume the helpers; create-app Template Sync Checklist updated (dropped the now-nonexistent instrumentation.node.ts row). Docs (README): new "Adopting in an existing app" section (manual Scenario-B steps) + /nextjs API table; corrected a stale "worker spans run through the no-op provider" note. Tests: nextjs.test.ts (externals completeness, recordHttpDuration semconv shape, register no-op + edge-skip). telemetry 50->54. Validation: build:packages 22/22, typecheck 22/22 (app + template + telemetry), lint clean, create-app 61/61 (byte-identity dispatcher sync still green). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(cli): add `mercato telemetry init` to wire telemetry into existing apps The web-tier telemetry wiring lives in app-owned source files, so a dependency bump can't deliver it to an app scaffolded before telemetry existed. Add a bootstrap-free command that patches it in — deterministic, idempotent, and --dry-run-able. Chosen over an agent skill because after the /nextjs surface reduction the work is mostly mechanical, so a unit-testable command with no AI-tool dependency fits; modeled on `mercato agentic:init` and `deploy railway --write-env`. packages/cli/src/lib/telemetry-init.ts (dispatched from mercato.ts, added to bin.ts BOOTSTRAP_FREE_COMMANDS) validates it's an OM app then applies six idempotent steps: - package.json: add @open-mercato/telemetry pinned to the app's existing @open-mercato version + optional bullmq-otel. - .env.example (+ .env if present): append the commented TELEMETRY_*/OTEL_* block when TELEMETRY_BACKEND is absent (detect-before-append). - src/instrumentation.ts: create if missing, else insert the guarded registerTelemetryForNextjs() block into register(). - next.config.ts: ts-morph confirms serverExternalPackages is a real array literal, then a formatting-preserving text splice adds the import + ...telemetryServerExternalPackages. - the API dispatcher: auto-patch via anchored insertion (imports + success-path recordHttpDuration + catch reportError/500 metric) ONLY when the known scaffold shape is recognized; otherwise print the snippet and flag a manual step rather than editing an unrecognized handler. run() no longer auto-creates .env for the telemetry command (excluded from ensureEnvLoaded like deploy). The env block + instrumentation content are duplicated from the template into telemetry-init.ts — noted in the create-app Template Sync Checklist. Tests: telemetry-init.test.ts (app-guard, full pre-telemetry wire, idempotent re-run with no double-insert, customized-dispatcher manual fallback, dry-run writes nothing). Full CLI suite 992/992, cli typecheck + build green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(cli): harden telemetry-init coverage; fix single-line serverExternalPackages Expand telemetry-init tests to prove the command actually works, not just that strings were inserted: - syntactic validity: every patched file (next.config.ts, instrumentation.ts, dispatcher) is parsed with the TypeScript compiler and asserted to have zero syntax errors. - live-template round-trip: strip telemetry from the REAL scaffold dispatcher, re-apply, and assert it reproduces the shipped wiring — ties the test to the template so it fails if either drifts. - no-op on the real already-wired template files (detection matches what ships). - variation coverage: recognizable-but-modified dispatcher (custom code preserved), unrecognizable dispatcher (untouched + manual snippet), next.config single-line AND multi-line arrays, serverExternalPackages absent (manual), and a pre-existing custom instrumentation.ts (body preserved). The expanded suite caught a real bug: the next.config spread insertion only matched the multi-line array form, so a single-line `serverExternalPackages: ['esbuild']` was half-patched (import added, spread missing). Fixed by splicing at the ts-morph array node's byte offsets instead of an outer regex — handles both forms and stays formatting-clean. telemetry-init 12/12, full cli suite 999/999, typecheck + build green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(telemetry): redact secrets in reported context, not just emails in text Addresses PR open-mercato#25 review: the redaction backstop only scrubbed emails from error text, so a secret in the reported context (e.g. an Authorization header attribute) or an inline auth token in a message would still ship to the backend. - redactPii now also masks Bearer/Basic tokens and Authorization/Cookie header dumps embedded in free text (message + stack). - new redactAttributes(): masks values under a secret-looking KEY (authorization, set-cookie, client_secret, x-api-key, access_token, …; specific enough that token_count survives) and runs redactPii over other string values. - applied at the single writeRecord log chokepoint, so it covers reportError context AND every logger.* attribute bag — mirroring how serializeError already runs error text through redactPii. Declined the paired "use Zod for the email regex" note: Zod validates whole values, but redaction needs substring scan/replace over free text; the always-loaded facade also avoids heavy deps. Tests: redact.test.ts (auth-token/header text, secret-keyed attributes, token_count preserved) + reportError-context e2e in telemetry.test.ts. telemetry 54->61; typecheck (telemetry/queue/app) + build green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(telemetry,cli,queue): honor .env for CLI telemetry init; flush on worker shutdown Two confirmed review findings (Kamil) + one suggestion (Patryk) on PR open-mercato#25. 1. CLI-launched processes ignored TELEMETRY_* set only in .env. bin.ts's static telemetry import evaluated the logger, whose module-scope readTelemetryEnv() stamped the env cache before any .env load; initTelemetry() then consumed the stale snapshot (backend -> noop) and run()'s later ensureEnvLoaded() came too late. So `mercato queue worker` with the backend only in .env — the exact path .env.example and `telemetry init` point users at — silently emitted nothing. Fixed at all three layers: - logger resolves env lazily per write (pino keyed on the memoized env object, rebuilt after a cache reset) — no import-time stamp; - initTelemetry() resets the env cache so init reads the fully-loaded environment; - bin.ts pre-loads the app's .env via a new bootstrap-free lib/load-env.ts (resolver + dotenv only, no pg — preserves the instrumentation load-order guarantee) before initTelemetry(). Verified e2e: a fixture app with TELEMETRY_BACKEND=console only in .env now logs `telemetry initialized {backend: console}` from the worker entry. 2. Worker graceful shutdown never flushed telemetry: the SIGTERM/SIGINT handler closed queues then process.exit(), and a worker never returns from run(), so bin.ts's post-run shutdownTelemetry() was unreachable — the BatchSpanProcessor's ~5s tail was dropped on every restart/redeploy. The handler now awaits shutdownTelemetry() (failure logged, never fails the shutdown) after queue close, before exit. 3. Add ApiKey to the auth-scheme text-redaction pattern (review suggestion). Also guard the optional provider.activeTraceContext?.() in writeRecord — a custom provider omitting it crashed every log write. Tests: env-load-order.test.ts (backend/log-level set after facade import must win), worker-shutdown-telemetry.test.ts (SIGTERM flushes before exit), load-env.test.ts, ApiKey redact case. telemetry 63, queue 59, cli 1002 green; typecheck + builds green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(telemetry): make spec and test docstrings self-contained The spec changelog and three new test docstrings referenced the internal fork review (PR number, reviewer attributions) — meaningless and dead links once this lands upstream. Reworded as self-contained defect descriptions; technical content unchanged. Comment-only change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(app): assert dispatcher telemetry at the recordHttpDuration seam The api-telemetry test still mocked histogram from @open-mercato/telemetry, but the dispatcher now records the http.server.request.duration metric via recordHttpDuration from @open-mercato/telemetry/nextjs, whose internal histogram import is relative and bypasses the package mock — so no calls were captured. Mock the /nextjs entry point instead and assert the method/route/status wiring; the semconv histogram shape is covered by the telemetry package's own nextjs tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(telemetry): inherit the shared jest base config The memory fan-out guard (scripts/__tests__/jest-memory-fanout.test.mjs) requires every package jest config to inherit the bounded maxWorkers and workerIdleMemoryLimit caps from jest.config.base.cjs; the telemetry config predated that guard on this branch and defined a standalone config. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(telemetry,queue,cli): close review gaps in the telemetry test suite Fixes from an adversarial review of every test file on this branch: - queue/async.telemetry: restore TELEMETRY_BACKEND after the suite — the module-scope 'otlp' leaked into the shared process.env and broke sibling queue suites whenever jest's file ordering shifted. - telemetry/nextjs: derive the expected serverExternalPackages set from the provider's real @opentelemetry/* imports instead of a second hand-maintained subset, and assert set-equality both ways. - telemetry/pg-instrumentation: probe the provider's DEFAULT instrumentation list (previously the child injected its own PgInstrumentation, leaving the production wiring unguarded) and assert bound query parameters never appear in span attributes. - telemetry/telemetry: the 'awaits async work' test was synchronous; it now drives an async callback through withSpan and asserts the span ends only after the awaited work completes. - cli/load-env: add the missing guard for the documented regression — bin.ts must await loadAppEnv() before initTelemetry(). - telemetry/env-load-order: reset the globalThis-keyed active provider between tests; logger-level: correct a stale import-time-binding docstring; queue/worker-shutdown: drop an unasserted claim from the test name. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(telemetry): re-raise SIGTERM/SIGINT after the Next.js flush Registering a signal listener suppresses Node's default termination, so registerTelemetryForNextjs()'s flush-only handler left the web process alive after SIGTERM until the orchestrator force-killed it. The once- installed handler now awaits the best-effort flush and re-raises the signal, restoring default terminate semantics. Regression-locked by nextjs-shutdown.test.ts: a spawned child registers the helper, receives a real SIGTERM, and must exit with that signal — verified to fail against the pre-fix build. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(docker): copy the telemetry package manifest in the dependency layer The Dockerfile now installs from per-workspace package.json copies (dockerfile-runtime-copy guard scans packages/*), so the new packages/telemetry workspace must be copied before yarn install in both stages. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(queue,telemetry): satisfy the structured-logging console gate Upstream's logger:check-console:ci gate (structured-logging facade, merged from develop) flags raw console.* calls. Convert the two queue call sites to the package logger already in scope, and allowlist telemetry's nextjs.ts init-failure warn: that module must stay next.config-eval-safe, the package has no @open-mercato/shared dependency, and the warn fires exactly when telemetry init failed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(telemetry): unify logging and harden opt-in loading * fix(telemetry): bound email redaction matching * fix(telemetry): upgrade OpenTelemetry past Jaeger advisory * fix(docker): include telemetry workspace in runtime focus * test(sales): seed required lines in ledger route cases * test(record-locks): seed valid sales orders * test(payment-gateways): create valid reconciliation orders * test(sales): repair order-line integration fixtures --------- Co-authored-by: Jakub Birecki <kubabir@outlook.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
zielivia
pushed a commit
that referenced
this pull request
Aug 26, 2026
…open-mercato#4391) * feat(channel-discord): scaffold provider package (adapter, REST client, credentials, health, DI) Adds the @open-mercato/channel-discord package implementing the existing communication_channels ChannelAdapter contract for a Discord bot: - DiscordChannelAdapter (send/edit/delete/reactions via REST, resolveContact, fetchHistory backfill, validateCredentials, fail-closed verifyWebhook) - Thin fetch-based Discord REST client (no discord.js), swappable in tests - Ed25519 interactions verification via node:crypto (no tweetnacl) - Credential + channel-state zod schemas, capabilities, health check - integration.ts / di.ts / setup.ts / acl.ts registration - Registered in app + create-app template modules.ts Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(channel-discord): gateway worker + signed interactions route - Discord Gateway WebSocket client (native WebSocket, no ws dep) with identify/resume/heartbeat state machine + pure testable helpers - Long-running gateway worker bridging MESSAGE_CREATE / reactions into the hub's existing inbound + reactions queues (no hub change); bot-self-message feedback-loop guard; requires_reauth on fatal close codes; honours OM_CHANNEL_DISCORD_GATEWAY_DISABLED - Provider-owned signed Interactions route (Ed25519 fail-closed, synchronous PING->PONG) resolving the spec's one hub touch-point without touching the hub - Pure gateway-bridge + interactions-handler helpers for unit testing Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(channel-discord): optional AI auto-reply subscriber - Listens to communication_channels.message.received (provider discord) - Soft-resolves ai_assistant via DI (mcpToolRegistry); no-ops when the peer is absent so the channel still works as a plain inbox (module-decoupling) - Easy-vs-complex classifier (SPEC-056 tiering): easy auto-replies text-only, complex is propose-only (no auto-send); default OFF per channel - Drafts via runAiAgentObject (dynamic import, optional peer) and sends through the generic hub outbound path (compose -> outbound-bridge -> deliver_outbound) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(channel-discord): connect widget + i18n (en/pl/de/es) - Profile connect injection widget (bot token, application id, public key, guild id, default channel id) via the shared credential connect route - injection-table wiring into profile:communication-channels:connect - i18n locale files (en/pl/de/es) for all user-facing connect strings; no hardcoded user-facing text Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(channel-discord): unit tests (Ed25519, mapping, sendMessage, gateway, subscriber) 57 unit tests covering: - Ed25519 interaction verification: valid / tampered body / tampered timestamp / missing signature / wrong key -> all fail-closed - Interactions handler: PING->PONG, tampered->401, tenant isolation (public-key pinning), no-candidate->401 - Discord message -> hub NormalizedInboundMessage mapping + bot-self filter - sendMessage builds the correct REST createMessage request (channel resolution, 2000-char clamp, defaultChannelId fallback) + validateCredentials - convertOutbound markdown passthrough / html down-convert / clamp / mentions - gateway state-machine helpers (intents, identify/resume/heartbeat, backoff, fatal close codes) + gateway->hub bridge job building - ai-reply classification + subscriber no-op when ai_assistant absent Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore(channel-discord): link workspace package (yarn.lock) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(channel-discord): gateway start trigger (CLI) + connection reconciliation Review blockers #1 and #2: - Add channel_discord 'start-gateway' CLI: enqueues the bootstrap job, runs the gateway worker, and re-enqueues a refresh job on an interval so newly connected channels are picked up and deactivated/soft-deleted ones are reconciled away. Inbound was otherwise silently dead (realtimePush disables hub polling and nothing enqueued the channel_discord_gateway job). Note the requirement in integration.ts description. - Full connection reconciliation each run: close + drop sockets whose channel left the active set (isActive=false / soft-delete) instead of leaking the socket + heartbeat timer forever. Track tenantId per connection so a scoped refresh never tears down another tenant's sockets. Exported pure reconcileGatewayConnections + unit tests (stale close, active keep, tenant isolation). - Align integration.ts version with package.json (0.6.6). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(channel-discord): heartbeat-ACK zombie detection on the gateway Review non-blocker #4: handle gateway opcode 11 (Heartbeat ACK). A missing ACK before the next heartbeat means a zombied connection (TCP up, gateway dead) — force a close(4000) so the close handler reconnects (with resume) instead of staying silently deaf. Add a pure createHeartbeatMonitor state machine (onBeat/onAck/reset) wired into the session + unit tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(channel-discord): drop ALL bot-authored inbound messages Review non-blocker #5: the feedback-loop guard now drops any message flagged author.bot (other bots / webhooks in the channel), not only our own bot's user id — prevents cross-bot loops and AI auto-reply noise. Own-bot id check retained as a belt-and-suspenders fallback. Mapping test updated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(channel-discord): full AI auto-reply subscriber gating tests Review blocker #3: exercise the whole subscriber with mocks — - easy -> drafts (runAiAgentObject) + sends via messages.messages.compose - complex/keyword -> propose-only, NEVER calls the agent or sends - ai_assistant absent -> clean no-op, message never loaded, nothing sent - per-channel auto-reply OFF (default) -> no-op - channel loaded scoped by tenant + organization Nails down the 'never auto-sends privileged/complex' guarantee at the subscriber. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(channel-discord): reject replayed interaction signatures (timestamp freshness) Discord signs timestamp+body, but a captured request stays cryptographically valid forever. Enforce a +/-300s freshness window (DISCORD_SIGNATURE_MAX_SKEW_ SECONDS) before the per-candidate Ed25519 fan-out, so a replayed capture is rejected at constant cost with 401 stale_timestamp. Fail-closed on missing or non-numeric timestamps; clock injectable for tests. Addresses the review's 'interactions replay / timestamp freshness' follow-up. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(channel-discord): harden easy/complex classifier against prompt injection The auto-reply gate matched keywords against raw text only, so cheap obfuscation (fullwidth chars, zero-width splits, markdown emphasis/spoilers, combining-mark zalgo) or a steering prompt could slip past it into the auto-send path. Now: - signals match a normalized copy too (NFKC, invisible chars stripped, markdown emphasis stripped, NFKD combining marks stripped) - injection attempts (ignore instructions, system prompt, act as, role markers, ...) force complex -> propose-only - messages carrying links are propose-only (unvetted instruction channel) - any zero-width/bidi character is treated as obfuscation -> propose-only Defense-in-depth over the existing containment (propose-only, features: [], allowed_mentions parse: []), per the review's hardening follow-up. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(channel-discord): sort i18n keys so the CI i18n-sync gate passes `yarn i18n:check-sync` requires flattened, sorted locale files. The four channel_discord locale files were unsorted, which turned the `test` job red once the base merge un-skipped it (it had been skipped behind the `audit` gate). Reordering only — key count is unchanged at 15 per locale. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(docker): copy the channel-discord manifest before the source copy The Dockerfile pre-copies every workspace package.json so yarn can install from manifests before the full source copy. packages/channel-discord was added by this PR without its three entries, which fails the dockerfile-runtime-copy guard and would break the layer cache for the new package. Adds the manifest copy to both dependency-install stages and the runtime stage. * fix(channel-discord): stop gateway churn and scope resume-state writes Two defects in the gateway worker, both reported in review. Churn: the worker restarted every active channel on every job run, and the CLI's 60s refresh made that a re-IDENTIFY loop — healthy sockets were closed and replaced once a minute, burning Discord's session-start budget and dropping the dispatches in flight. The refresh is now a reconciler: a channel whose session is still running is left untouched and only a stopped handle is replaced. `DiscordGatewayHandle` gains `isActive()` so the worker can tell a session that is merely reconnecting with backoff (self-healing) from one that is really dead. Opcode 9 (INVALID_SESSION) previously cleared the resume state and returned, leaving the socket open but permanently deaf: no dispatches, no reconnect, no error. It now honours Discord's `resumable` flag — clearing the session only when it is not resumable — and closes the socket so the existing close handler runs the shared backoff plus the correct IDENTIFY/RESUME handshake. Scoping: resume state was persisted with an unscoped `findOne({ id })` and a flush issued straight from a socket callback. Writes now go through a provider-local store that filters by `tenant_id` / `organization_id`, merges only the gateway-owned keys (so a concurrent operator edit of `aiAutoReplyEnabled` is not clobbered), refuses a patch that would rewind the sequence of the session already stored, and runs on its own EntityManager fork. The worker serializes those writes per channel so two callbacks cannot interleave their read-modify-write. * fix(channel-discord): declare only the capabilities the adapter implements The capability profile advertised file sharing, typing indicators, presence, rich blocks, interactive components, inline images and stickers, none of which this adapter implements: `convertOutbound` drops `content.attachments`, `discord-rest` has no multipart upload, the bot identifies without the `GUILD_PRESENCES` intent, outbound is plain markdown `content`, and the Interactions endpoint answers with a deferred ack and never follows up. The hub routes work to the adapter based on these flags, so each one is now set to what actually works today; `threading`, `richText`, `reactions`, `editMessage`, `deleteMessage`, `conversationHistory` and `realtimePush` stay enabled because each is backed by a real adapter method. The doc comment names the code behind every enabled flag and what has to land before a disabled one may flip. `convertOutbound` now logs and reports `droppedAttachmentCount` instead of discarding attachments silently, so a mis-routed attachment is observable. * fix(mercato): declare the channel-discord dependency the app registers `apps/mercato/src/modules.ts` registers `@open-mercato/channel-discord` but the app's `package.json` never declared it, so resolution worked only through workspace hoisting and would break under an isolated install. * feat(channel-discord): add the provider-owned env preconfiguration the spec requires SPEC 2026-06-19 promises deployment-managed bootstrap via `OM_CHANNEL_DISCORD_*` applied from `setup.ts` and rerunnable through a provider CLI command; `setup.ts` only registered the adapter. `lib/preset.ts` reads the vars and persists them through the standard `integrationCredentialsService` (never a core special case). A half-filled preset throws instead of persisting a bot token that cannot authenticate, and existing credentials — the normal `/backend/integrations` connect flow — are kept unless `OM_CHANNEL_DISCORD_FORCE_PRECONFIGURE` says otherwise, so both entry points are idempotent. `setup.ts` applies it on tenant creation and `yarn mercato channel_discord configure-from-env --tenant <id> --org <id>` re-applies it to tenants that predate the vars or after a token rotation. Both `.env.example` files document the vars (create-app template sync), and the `start-gateway` doc comment now states that the periodic refresh is a reconciler, not a re-connector. The standalone scaffold keeps the package installed but leaves `channel_discord` commented out in `template/src/modules.ts`: enabling it makes a 49th entry in the Codex root's enabled-module fact index, which pushes the generated AGENTS.md past its 12 KiB `project_doc_max_bytes` budget. That budget review is a maintainer call, so the line ships ready to uncomment rather than red. * fix(channel-discord): serve the interactions endpoint at its documented URL The route file sat at `api/post/channel_discord/interactions/route.ts`, but the API generator already prefixes the module id — so the endpoint was registered at `/api/channel_discord/channel_discord/interactions` while the route's own doc comment, `integration.ts` and the operator instructions all say `/api/channel_discord/interactions`. An operator following the documentation would point Discord's Interactions Endpoint URL at a 404 and the mandatory PING handshake would never complete, so slash commands and buttons could not be enabled at all. Moved the file under `api/post/interactions/` and pinned `metadata.path` explicitly, the way the hub's `webhooks/gmail` route does: the operator-facing URL is part of this route's contract, not a by-product of where the file sits. Caught by TC-CHANNEL-DISCORD-005/008, which no unit test could have caught — the mismatch only exists in the generated route manifest. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(channel-discord): ship the spec's integration coverage (TC-001..008) Review finding: the spec requires TC-CHANNEL-DISCORD-001..010 to ship with the implementation and its compliance report claimed they did, but the package held only mocked jest tests — no module-local `__integration__` suite. Eight executable Playwright specs now drive the real app: - 001 the `discord` adapter is registered (an unknown provider key 404s, discord 422s) and `validateCredentials` fails closed offline with per-field errors; a rejected connect leaves no half-created channel - 002 test-send refuses unauthenticated callers, malformed ids and channels the caller does not own, before any adapter resolves - 003 a `providerKey: 'discord'` inbound message persists as a delivered inbound link inside the channel's health window - 004 reaction add/remove round-trip on a Discord message through the hub's thread mapping - 005 the signed interactions route is fail-closed on every path: unsigned, fresh-timestamp-without-signature, unknown key, tampered body, replayed timestamp (the freshness guard answers before the candidate fan-out), non-JSON - 006 an inbound Discord message from a known contact creates EXACTLY one CRM interaction — the provider adds no contact-resolution logic of its own - 007 the health surface is tenant-scoped and reports a fixed numeric snapshot that counts Discord traffic - 008 the shared interactions URL is a black box for an unverified caller: identical rejections across applications, no channel/tenant/organization identifier in the body Ed25519 request signing is generated per test (`helpers/discordSignature.ts`), so no Discord application, bot token or network call is involved. Specs that need seeded traffic use the hub's env-gated fixture and skip with a stated reason when `OM_ENABLE_TEST_CHANNEL_SEEDING` is off, matching the hub's own `TC-CHANNEL-API-*` convention; 001, 002, 005, 008 and 007's guards need no fixture. Each spec states the ceiling of what is assertable without a live Discord application and names the unit test that owns the other half. The spec's compliance report and a new section record that honestly, and TC-009/TC-010 move with the AI auto-reply feature they assert on (#4778). Verified: 20/20 pass against an ephemeral app + Postgres. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(channel-discord): de-scope AI auto-reply from the first release The 2026-07-30 re-review established that the subscriber cannot invoke any agent this repository ships (object-mode call with empty features against chat-mode, feature-gated agents) and that nothing writes the channel-state keys that arm it. Rather than design AI invocation underneath a provider PR, the first release stops promising the capability: the module and integration descriptions no longer advertise it, the `ai` tag is gone, and the subscriber, its helpers and the channel-state keys document that they ship dormant and why arming them by hand would still be denied. The complex tier's log line no longer claims an approval surface that does not exist. Production agent invocation, the configuration path, the proposal surface and TC-CHANNEL-DISCORD-009/010 are tracked in #4778. No behavior change. * fix(channel-discord): align react and ts-jest devDependencies with the monorepo packages/channel-discord pinned react/react-dom 19.2.7 and ts-jest ^29.4.11 while every other workspace package (and the committed lockfile) is on react/react-dom 19.2.8 and ts-jest ^29.4.12. That divergence made `yarn install --immutable` want to add react@19.2.7 / react-dom@19.2.7 resolutions and split the ts-jest descriptor, so YN0028 failed the install step and took prepare, lint and audit down with it. Match the sibling channel packages (channel-gmail, channel-imap) and regenerate the lockfile; the only lockfile change is the workspace descriptor block, no new resolutions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(channel-discord): add the ko locale the base now requires develop added Korean as a fifth locale; yarn i18n:check-sync fails the test job for any module missing ko.json. Translates the 15 existing keys rather than stubbing them with English, matching the sibling modules' ko files. * fix(channel-discord): quarantine fatally-rejected channels, fail fast on a local queue Addresses the autofixable half of @Kapsik89's QA report on #4391. The two functional blockers (#4975 inbound rejected, #4976 no send path) are NOT touched here — both need a maintainer decision on whether the hub keeps requiring an email address to identify an external correspondent. #4979 — a channel Discord fatally rejected (4004 bad token, 4014 disallowed intents) is now persisted as `requires_reauth` with the close code as `last_error`, and the reconciler skips it. Previously `onRequiresReauth` only emitted an event nothing subscribes to for state changes, so every tick saw "no live session" and IDENTIFYed again: 16 fatal closes over 10 ticks at `--refresh 5`, ~1440 session starts/day at the default refresh against Discord's ~1000/day per-bot budget, with the row still reading `connected`. #4978 — `start-gateway` refuses to start unless `QUEUE_STRATEGY=async`. It runs a worker for the gateway queue only while the bridge enqueues into the communication-channels queues served by the app server process, so under the default in-process strategy inbound jobs were dropped with no error, no log and no retry. The inbound path also logs receive / dropped-as-bot-authored / enqueued, which is what made a dead socket indistinguishable from a lost job during QA. #4977 (interim) — a bot token already served by a live session cannot open a second socket. The real fix is hub-side: the hub derives `externalIdentifier` from email-shaped credential keys, which Discord has none of, so every reconnect inserts a duplicate channel row. Guarding here keeps the single-identify-per-bot discipline until that is decided. The token is matched by SHA-256 fingerprint, never stored or logged raw. #4982 — provider description cut from 396 to 108 characters, inside the 68-132 range its six siblings occupy, so the card stops inflating its whole grid row; literal backticks removed and ASCII arrows replaced with proper ones. #4983 — the create-app template blocker now has an owner: #4986 tracks the generated AGENTS.md hitting its 12 KiB budget at 49 enabled modules. TC-CHANNEL-DISCORD-003 no longer claims to prove that Discord inbound works. It passed by feeding the hub an invented @test-seed.local address for both `externalIdentifier` and `to`, so `normalizeInboundDiscordMessage` never ran on that path. The sender-identity assertions it pretended to make now live in a unit test against a real MESSAGE_CREATE frame, and the end-to-end path is an explicit `test.fixme` referencing #4975. Package suite: 124 passing, up from 106. * test(cli): restore headroom in the module-facts size guard The `test` job was red on one assertion: the whole-repo module-facts JSON render measured 3,504,040 bytes against a 3,500,000 cap. Measured on this tree, the guard was already exhausted before this PR's module existed — 3,488,120 of 3,500,000 JSON bytes (99.66%) and 1,520,711 of 1,550,000 markdown bytes (98.11%) come from the 55 modules on develop. A whole communication-channel provider costs 15,920 JSON and 9,239 markdown bytes (`channel_gmail` 6,886, `channel_imap` 6,798; the gap is a gateway worker, a CLI command, a signed route and a subscriber, spread proportionally across overrideTargets, extensionSurfaces, factSources and ownedContracts — nothing duplicated). So both caps had stopped detecting blow-ups and started rejecting whichever PR happened to add the next module, of any size. Raised to ~20% headroom above the current tree, with the measurement recorded inline next to the three earlier raises. The guard keeps its purpose: a real blow-up here is multiplicative — a duplicated provenance payload, or a contribution body copied per resolution — not one provider's worth of references. The delta cap (1,800,000, currently 1,527,947) still has 15% headroom and is unchanged. Co-authored-by: Claude Opus 5 <noreply@anthropic.com> * test(channel-discord): make the AI auto-reply dormancy claim executable Review round 2 (@pkarw, 2026-07-30) ended on the coverage verdict: the provider suite "mocks away the central AI policy/runtime". The release answered the capability half by de-scoping AI auto-reply to #4778, but the coverage kept the shape the review objected to — a stubbed `runAiAgentObject` under a test named "drafts and sends", which reads as proof of a capability this release does not promise. Two changes, no behaviour change: - `ai-auto-reply.dormancy.test.ts` turns the PR description's prose claim ("nothing in the product writes the two arming keys") into a guard. It scans the package's own sources and fails if `aiAutoReplyEnabled` / `aiAgentId` are ever named outside the four files that read them, or by any widget, API route, setup hook, preset, CLI command or integration descriptor. Verified by violation: dropping a probe file naming a key under `widgets/` fails both assertions. - The existing subscriber test says what its stub does and does not prove, and stops claiming a capability: the agent call is stubbed because `@open-mercato/ai-assistant` is a genuinely soft-optional peer (absent from dependencies/peerDependencies — hence `{ virtual: true }`), so importing the real runtime here would break the decoupling property the review verified. Under the real policy the `features: []` call this subscriber makes is refused for every chat-mode, feature-gated agent the repository ships; the coverage that drives that policy for real belongs with the capability, in #4778. Package suite 127 passing, up from 124. Co-authored-by: Claude Opus 5 <noreply@anthropic.com> * docs(create-app): record the measured cost of enabling channel_discord (#4983) #4986 closed with #4989, which removed the hard overflow this comment used to cite: the generated standalone root now sheds its inline module-fact index for a pointer form instead of blowing the 12,288-byte target. Enabling the module here was therefore re-tested rather than assumed, and it still does not pay off — measured on this head, the generated classic root lands at 12,275 bytes with the inline index intact, i.e. 13 bytes of headroom, so the next module enabled after it drops the index to pointer form and fails the guard test that protects it. Per packages/create-app/AGENTS.md the fallback is something to accept deliberately or to pay for by reclaiming root bytes, and neither is this PR's call to make. The comment now states the measurement and points at the guard test instead of at a closed issue, which is what #4983 actually asked for: an owner and a reason, not a bare TODO. * fix(channel-discord): align the package version with the monorepo (0.6.7) The base merge brought in the 0.6.6 → 0.6.7 bump, which touched every public package except this one — it does not exist on `develop` yet, so the release bump had nothing to rewrite here. `scripts/check-version-alignment` is exactly the guard for that case and it went red on the merged head: `@open-mercato/channel-discord@0.6.6 (expected 0.6.7)`. Worth noting for the local gate: this guard lives in root `scripts/__tests__`, which turbo never picks up, so `yarn test` is green while CI's separate `yarn test:scripts` step fails. Verified here with `yarn test:scripts` (475 passing) rather than by re-running `yarn test`. Version-only change; no code, no dependency shape. * test(cli): record the measured fact budgets behind the merged bc-guard caps * chore(channel-discord): pin the two peers every sibling channel package pins `yarn test:scripts` went red on the merged tree with two unmet peers for `@open-mercato/channel-discord`: `@open-mercato/core` requires `@open-mercato/ai-assistant`, and `@open-mercato/ui` requires `react-is`. Neither is specific to this provider. Every sibling — `channel-gmail`, `channel-imap`, `channel-apns`, `channel-expo`, `channel-fcm`, `checkout` — has the identical pair pinned with the identical reasons; `channel-discord`'s package.json declares exactly the same dependencies and peerDependencies as `channel-apns`. It is absent from the allowlist only because it did not exist when `develop` added the guard, so the merge is the first time the two met. That is precisely the case the guard documents itself as handling: pre-existing unmet peers are pinned so the gate blocks NEW ones rather than demanding an unrelated cleanup first. Generated with the script's own `--update-allowlist`, then reworded to match the siblings verbatim so the file stays readable and the follow-up note keeps applying to every entry that shares it. * test(channel-discord): make TC-CHANNEL-DISCORD-003 assert what its name claims The spec was a `test.fixme` plus a second case that fed the hub an invented `…@test-seed.local` address for both the channel identifier and the recipient. That is why it stayed green through three live defects: a real Discord channel carries `external_identifier = NULL` and a real Discord sender has no address, so `normalizeInboundDiscordMessage` never ran on the asserted path. Both things that blocked the honest version have landed. #5252 made the `externalEmail` requirement conditional on the originating channel being email-typed, and the test-seed `ingest-inbound` action drives the real `ingest_inbound_message` command, which composes through `messages.messages.compose` instead of inserting rows behind it. So the spec now starts from a verbatim `MESSAGE_CREATE` frame, runs it through the **real** `normalizeInboundDiscordMessage` — the same function the gateway worker calls, not a re-implementation — and hands the result to the hub with no address anywhere. It asserts the sender stays a snowflake, that the normalizer invents nothing, and that a platform message and `MessageChannelLink` exist at the other end. Before #4975 that last step failed validation, retried three times and died in the queue while the channel still reported `Connected`. The ceiling is stated in the file rather than implied: the transport is the chat-flavoured test-seed adapter, because connecting a real Discord channel needs a live bot token and a live credential probe. What is not stubbed is the part that was broken — the frame, the normalizer, and compose validation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(channel-discord): declare provider-native recipients so outbound actually works Found in QA of this PR against a live Discord bot, on a tree that already has both #5252 and #5261 merged. Posting a genuine Discord channel snowflake to `POST /channels/{id}/test-send` still answers: HTTP 422 {"error":"Recipient must be a valid email address"} #5261 built the mechanism that was supposed to fix #4976 — recipients are validated against the adapter's declared `capabilities.recipientFormat` — but nothing wired Discord into it. `grep recipientFormat` across the whole repository finds the type in `lib/adapter.ts`, the check in `lib/outbound-recipient.ts`, `'email'` in `lib/email-capabilities.ts`, and nothing else. No adapter anywhere declares `'provider-native'`, so that branch is unreachable in production and only its own unit tests — which pass a literal `{ recipientFormat: 'provider-native' }` object — ever execute it. The consequence is that #4976 is still live for the provider it was filed against: `validateOutboundRecipient` falls through to the email default and rejects every real Discord recipient. The declaration belongs here rather than in the hub: the adapter owns the shape of its own recipients. With it, the hub applies transport safety only (the allowlist and length ceiling #5261 added, which is the part that matters — Discord interpolates the recipient into `/channels/{recipient}/messages`) and this adapter keeps treating the value as untrusted. The capability-contract test did not catch this because it pins feature flags, not `recipientFormat`; it now pins this too, so the branch cannot silently go unreachable again. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(channel-discord): post to the recipient the operator asked for Second finding from QA of this PR against a live Discord bot, and the more dangerous of the two because it looks like success. `resolveTargetChannelId` read `metadata.discordChannelId`, then `conversationId`, then `defaultChannelId`. It never read `metadata.to` — which is exactly where the hub's `test-send` route puts the operator's recipient: metadata: { to: body.to, subject: ..., testSend: true } So every test send went to `defaultChannelId` regardless of what was asked for, and the endpoint answered `200 sent`. Proven live: posting with `to: "999999999999999999"` (a channel id that does not exist) returned `{"status":"sent","externalMessageId":"1541054518630817792"}`, and fetching that id from Discord shows the message sitting in the default channel. An operator test-sending to one channel gets a green result for a message that landed in another; a recipient that is validated and then discarded is worse than one that is rejected. `conversationId` deliberately still outranks `metadata.to`: on a reply the thread mapping is authoritative and a stray recipient must never redirect a reply out of its own conversation. `metadata.to` decides only when there is no conversation, which is the test-send case. Both orderings are pinned by tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(runs): adopt PR #4391 — reconstruct execution plan Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(channel-discord): reject unsigned interactions before loading any candidate The Interactions endpoint is unauthenticated by design, so any caller can POST to it. It resolved every active Discord channel in the installation and decrypted each one's credentials BEFORE it validated anything about the request, so a POST with no headers and an empty body cost 1+N..1+2N database round-trips and N AES decrypts, where N is the number of Discord channels across the whole installation. The fail-closed semantics and the tenant pinning were correct; the ordering was not, and the "constant cost" claim in the handler's doc comment described an intent the code did not implement. Two changes, both local: - Hoist the request-only guards ahead of the candidate load. Timestamp freshness and the signature header's presence and hex shape depend on the request alone, so `screenInteractionRequest` decides them first and `resolveDiscordInteraction` only calls the candidate loader for a request that survives. The unsigned, malformed and replayed paths now touch the database zero times, which the new call-count assertions pin so the ordering cannot silently regress. The doc comment now describes what the code does. - Narrow the candidate set by the `application_id` the body claims before the Ed25519 fan-out, so the fan-out runs over the channels of one Discord application instead of every Discord channel. This is a NARROWING and never an authorization decision: the value is read before any signature is verified, it can only ever shrink the set, and a forged `application_id` still fails the signature gate — asserted explicitly. Credentials are also resolved once per (tenant, organization, user) scope rather than once per channel row, so channels sharing a credential bag no longer each pay for a decrypt. Response bodies and status codes are unchanged for every input. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(runs): mark discord-channel-provider Phase 2 complete Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(channel-discord): declare threading honestly, honour the job's organization scope Four review findings, all about the provider promising something it does not do. - `capabilities.threading` was `true` on the strength of `convertOutbound` emitting a `message_reference` from `channelMetadata.replyToExternalId`. No hub producer writes that key into OUTBOUND metadata: it exists only on the inbound `NormalizedInboundMessage` shape, and the hub's outbound producers (`send-as-user.ts`, `deliver-outbound-message.ts`) write the email-shaped `inReplyTo` / `references`. The branch was unreachable in production, as a live bot confirmed in #5541. Declared `false` with the reason naming the missing producer, pinned by a contract test that also asserts the conversion still works, so the flag flips back with that producer and nothing else. - The `defaultChannelId` help text still told operators the hub test-send endpoint validates its recipient as an email address and therefore could not exercise this. #4976 is fixed — #5261 built the mechanism and this branch declares `recipientFormat: 'provider-native'` — and QA sent a real message through it. (`helpText` is a plain English descriptor field across all seven providers; the module's locale files carry the connect-widget field labels, which are unchanged.) - The spec's Adapter method map still declared `fileSharing: true` and `interactiveComponents: true` against code declaring both `false`, and gave the subscriber `metadata.id` as `discord-ai-auto-reply` while the code ships `channel_discord:ai-auto-reply`. Subscriber ids drive dedup, so an id copied out of the spec would have been read as a different subscriber and re-delivered handled events. Both corrected, with a changelog entry. - `GatewayJobPayload.organizationId` was accepted and silently ignored: both the channel query and the reconciliation filtered on tenant alone, so a job scoped to one organization connected the whole tenant and — worse — reconciled away every sibling organization's live sockets, since none of their channels were in the set it compared against. Honoured now in both places, with the filter extracted to `buildGatewayChannelFilter` so the scoping is pinned directly. An explicit `organizationId: null` still means "no filter", not "tenant-wide rows only". Carries the three review nits too, since two of them touch the same files: - `integration.version` `0.6.6` → `1.0.0`. The old value was copied from the package's monorepo version, so it read as a claim to track it and had already drifted (the package is at 0.6.7). A comment now says what the field is for. - `start-gateway` used the positional-pairing `parseArgs`, where a bare `--flag` shifts every remaining pair. It now uses `parseFlagsAndValues`, which the sibling command already used, and the old parser is deleted. - `draftAndSendEasyReply` no longer takes the `channel` it never read, instead of suppressing the warning with `void channel`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(runs): mark discord-channel-provider Phases 3 and 4 complete Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(auth): add the Discord provider's ACL feature titles to the catalog `develop` gained a guard that walks every discovered `acl.ts` and requires each declared feature to carry a matching English title in the auth ACL translation catalog. This branch adds a module the guard had never seen, so merging develop turned it red: `channel_discord.view` and `channel_discord.configure` had no catalog entry at all, which in the product means the permissions UI would render the raw feature id instead of a name. Adds both titles, following the convention the five sibling channel providers already use in this catalog: English and Polish translated, German, Spanish and Korean carrying the English fallback. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(runs): mark discord-channel-provider Phase 5 complete Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(channel-discord): dispatch slash commands and components past the deferred ack (#4663) The Interactions endpoint verified Ed25519 signatures and then answered every application command and message component with a bare deferred acknowledgement. Nothing replaced it, so an operator who registered a slash command saw Discord's "thinking…" state forever, and the interaction never reached the hub. That was disclosed as deferred scope on #4391 and tracked as #4663; this closes it. Discord gives the endpoint three seconds, so the route still answers with a deferred ack — but now it hands the verified interaction to a dispatch worker first, and the ack is only returned once the hand-off is queued. The worker does the two slow halves: it normalizes the interaction into the hub's existing `communication-channels-inbound` queue and replaces the placeholder over Discord's interaction-webhook endpoints. The hub contract is untouched. The interaction is presented as a synthesized Discord message object and reaches the hub through `adapter.normalizeInbound`, so a slash command lands in the same conversation, under the same tenant scope, with the same `(channel_id, external_message_id)` dedup discipline as anything typed in the channel. `normalizeInboundDiscordMessage` adds the interaction id, type, command name and component custom id to `channelMetadata` so a consumer can tell a command from a chat message without re-parsing `channelPayload`. - `APPLICATION_COMMAND`, `MESSAGE_COMPONENT` and `MODAL_SUBMIT` are dispatched. - Autocomplete is answered synchronously with an empty choice list, the only response Discord accepts for it — a deferred ack there is rejected. - Any other type, and any payload missing the follow-up token, channel or invoking user, gets a visible ephemeral reply. The endpoint never returns a deferred ack it cannot redeem. - The follow-up edits the original response first, because that is the call that ends the "thinking…" state, and falls back to a follow-up POST when the original is gone. Neither carries the bot token: the interaction token in the URL is the credential. The queue payload carries a credential *scope*, never a secret, because the local queue strategy persists payloads to disk as JSON. - `register-slash-commands` ships as a CLI command. Guild-scoped, because guild registrations take effect immediately while global ones take up to an hour, and the call replaces the guild's list so re-running converges. `interactiveComponents` flips back to `true` in this same commit, guarded by a parity test in `capabilities.test.ts` that drives the whole path — dispatch, hub job, follow-up — rather than asserting the flag against itself. The capability profile cannot drift ahead of the implementation again without that test failing. The spec's setup section still pointed operators at `/api/communication_channels/webhook/discord` for the Interactions Endpoint URL; the shipped route is `/api/channel_discord/interactions`, and anyone who copied the old value would have failed the PING handshake. Fixed, along with the capability table, the module file table and the reused-hub-routes list. Refs #4391 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(channel_discord): ship the de-scoped AI auto-reply (#4778) PR #4391's re-review established that the AI auto-reply subscriber could not work against anything the repository shipped, and that no operator could turn it on. It called `runAiAgentObject` with `features: []`, every shipped agent was chat-mode and feature-gated, and nothing wrote the two channel-state keys that arm it. The feature was de-scoped and tracked here. This closes all four halves of that finding. A production agent. `channel_discord/ai-agents.ts` declares `channel_discord.auto_reply`: object-mode with its own `{ reply, summary, confidence, requiresHuman }` output schema, `readOnly`, `mutationPolicy: 'read-only'`, no allowed tools, gated on the real feature `channel_discord.ai_auto_reply.run`. Object mode is what makes propose-only mechanical rather than advisory — the runtime discards the resolved tool map before calling `generateObject`, so no tool is reachable from an inbound Discord message at all. A service principal with real features. `lib/ai-service-principal.ts` resolves the tenant's channel-bot user and loads its real ACL, falling back to a single code-declared, non-data grant when no such user exists. `isSuperAdmin` is clamped to false on both paths so a message from a public server can never borrow a super-admin's ACL. A configuration path. `GET`/`PUT /api/channel_discord/channels/{id}/ai-auto-reply` behind a `CrudForm` page, reached from an AI auto-reply panel on the Discord integration's detail page. Zod-validated, scoped and authorized through the hub's own org-scope and channel-access guards, mutation-guarded, optimistic -locked through the DI-aware seam, and persisted by a command that merges into `channelState` so a settings save cannot clobber the gateway's resume cursor. Enabling is refused when the AI peer is absent or the agent is one the runtime would reject. A proposal surface. The `complex` tier files an internal message carrying the drafted reply with approve/dismiss actions instead of logging and returning; approving composes the public reply attributed to the approving operator. With nobody assigned to the conversation the proposal is stored as a draft and the miss is logged. Auto-send now needs three independent yeses — the regex tiering says easy, the model sets `requiresHuman: false`, and its confidence clears 0.6. `@open-mercato/ai-assistant` is declared as an optional peer; the runtime coupling stays a dynamic import behind a DI presence check. The dormancy contract is replaced by an armed contract, and a new spec drives the REAL `agent-policy` / `agent-runtime` with only the model call stubbed. The provider advertises AI auto-reply again and the `ai` tag is back — in the same change that makes it true. Refs #4391 Closes #4778 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(channel-discord): move the API routes off the legacy method-folder layout The provider shipped its three routes under `api/get/`, `api/put/` and `api/post/`. That layout is the minority convention here (25 route files against 556 using `api/<path>/route.ts` with exported method handlers) and the generator itself scans it under a `// Legacy per-method` heading. Worse, a `route.ts` placed inside a method folder is picked up TWICE: once by the `route-file` scan, which walks all of `api/` including the method dirs, and once by the per-method scan, whose include filter accepts any `.ts`. Both entries resolve to the same `metadata.path`, so the manifest carried two registrations for every one of these routes. api/post/interactions/route.ts -> api/interactions/route.ts api/get/channels/[id]/ai-auto-reply/route.ts \ api/put/channels/[id]/ai-auto-reply/route.ts -> api/channels/[id]/ai-auto-reply/route.ts GET and PUT now live in one file with one `metadata` block declaring each method's own guard, and one merged `openApi`. The generated shard confirms the result: two entries, `kind: "route-file"`, `methods: ["POST"]` and `["GET","PUT"]`. No URL, guard, payload or behaviour changes — `/api/channel_discord/interactions` and `/api/channel_discord/channels/{id}/ai-auto-reply` are byte-identical, and the interactions route keeps its explicit `metadata.path` because that URL is operator-facing (it goes into Discord's Interactions Endpoint field); it now documents the derived path rather than overriding it. Also drops the legacy `export default POST` (the path-based layout reads named exports), and tightens the armed-contract test: it asserted the configure feature appeared somewhere in the write route's file, which a merged GET+PUT file would satisfy even if only the read half were guarded. It now matches the PUT metadata block. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(channel-discord): authorize the auto-reply agent at save time, and surface a dormant channel Addresses the three findings from the 2026-08-25 re-review of #4391. Medium — the settings route checked that the chosen agent was SHAPED right (object-mode, so `runAiAgentObject` would take it) and stopped there. The runtime asks a second question: `checkAgentPolicy` runs the agent's `requiredFeatures` against the principal from `lib/ai-service-principal.ts`, which carries one grant unless the tenant created a channel-bot user with a wider role. An operator could pick an agent from another module, the PUT would succeed, the UI would read "Auto-reply on", and every inbound message would be denied inside a background subscriber — the dormancy failure #4778 was filed for, wearing a green tag. - PUT resolves that same principal and refuses the save, naming the missing grants in the response body. The comparison runs through the platform's own `authorizeFeatures`, one feature at a time, so wildcard role grants (`customers.*`, `*`) resolve exactly as the runtime resolves them — a hand-rolled `Set.has` would reject a bot user whose role legitimately covers the agent. - Channel authorization moved ahead of the agent checks, so a caller who may not manage the channel gets the masked 404 before anything about the agent registry is disclosed. - GET marks every offered agent `invocable` / `missingFeatures`. The picker labels the ones the principal cannot invoke rather than silently offering them, and the form defaults to one it can. A save-time check goes stale the moment a role is edited, and the subscriber degrades every failure to a no-op by design, so "armed but answering nothing" also had to become observable rather than merely rarer: - Each failure writes `channelState.aiAutoReplyLastError`, through `lib/channel-state-store.ts` so the single-writer contract on the arming keys still holds, and the next successful attempt clears it. Repeats of the same reason do not rewrite the row, so a broken channel does not generate a row update per inbound message. - The settings page renders it as a banner; the integration panel flips the channel's tag from "Auto-reply on" to "Auto-reply failing". - The hub's own `channel.lastError` is deliberately untouched: the hub owns it for delivery and polling and clears it on a successful poll, so sharing it would let the two failure kinds erase each other. - New `lib/failure-reason.ts` redacts the stored reason. It leaves the log and enters the product, and an upstream SDK that echoes an Authorization header would otherwise put a live token in a JSONB column and on an operator's screen. Low — the AI auto-reply panel was an N+1 from the browser: it listed channels and then called the per-channel settings route once per channel, and each of those responses rebuilt the whole agent registry to render two booleans. New `GET /api/channel_discord/ai-auto-reply/channels` answers the panel in one query and never touches the directory; a test asserts that so the N+1 cannot creep back. The route pins its own `metadata.path`, the way the interactions route does, so the collection cannot be mistaken for a channel id. Low — `lib/interactions-queue.ts` read as a blanket "no credential travels on this payload" guarantee. True for the bot token, not for `interaction.token`, which can post as the application on its own and does land on disk under the local queue strategy. The comment now says so, with the 15-minute lifetime that bounds it. Tests: 258 unit tests in the package, up from 210 — a route-level arming matrix (refused agent, provider agent under the bare service principal, foreign agent once granted, wildcard grant, disarm, masked 404, absent peer, wrong shape), the feature comparison against the real `authorizeFeatures`, the marker's write/clear/dedup/truncation/scope rules, redaction, and proof that a marker write can never escalate a degraded no-op into a thrown handler. Spec and its changelog updated. No schema change, no hub contract change; the two new state keys ride the existing `channel_state` JSONB additively. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(channel-discord): drop the now-unused isDiscordEligibleAgentId `findDiscordEligibleAgent` replaced its only call site in the same change that added the requiredFeatures check, because that caller needs the agent's `requiredFeatures` too and the boolean form meant loading the agent registry twice. Leaving the wrapper behind would be dead exported code in a lib the package does not re-export publicly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: wojciechszyjka <wojciechszyjka@users.noreply.github.com> Co-authored-by: Piotr Karwatka <piotr.karwatka@gmail.com>
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
1 participant
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.
Summary
Adds SPEC-072 — UX enhancements for CRM detail pages (company, person, sales document), extending the existing SPEC-046 (customer detail v2) and SPEC-047 (sales document detail v2) with interaction improvements identified during the UX/UI meeting on 2 April 2026.
The spec addresses five critical UX gaps: information overload from non-collapsible form groups, high-friction activity creation (requires navigating to Zone 2 tab), single-owner limitation on entities, lack of visual deal progression, and unfiltered activity history.
Changes
.ai/specs/SPEC-072-2026-04-06-crm-detail-pages-ux-enhancements.md— full specification covering:customer_entity_rolesentity)Specification
Does a spec exist for this feature/module?
Spec file path:
.ai/specs/SPEC-072-2026-04-06-crm-detail-pages-ux-enhancements.mdRelated specs:
Testing
Spec-only PR — no code changes. Spec includes 5 integration test definitions (TC-UX-001 through TC-UX-005) to be implemented alongside the code.
Checklist
develop.docs/cla.md)..ai/qa/tests/(or documented why integration coverage is not required)..ai/specs/with a changelog entry (if applicable).Linked issues
Based on UX/UI meeting 2 April 2026 (Jarek, Oliwia, Maciej). Competitive analysis: Bitrix24, Pipe Drive, Tilio, Odoo.