diff --git a/backend/__tests__/unit/services/externalFeedService.test.js b/backend/__tests__/unit/services/externalFeedService.test.js index 0fe290c6eb..c8696521c5 100644 --- a/backend/__tests__/unit/services/externalFeedService.test.js +++ b/backend/__tests__/unit/services/externalFeedService.test.js @@ -232,6 +232,11 @@ describe('externalFeedService', () => { $set: expect.objectContaining({ status: 'error', errorMessage: 'invalid_request', + // Named, not omitted (wren 73858): a `$set` that leaves a key out + // inherits the row's stored value, so silence here would keep a stale + // `true` from an earlier human-authored reason and print this + // provider text in the user's connector list. + errorMessageUserFacing: false, }), }), ); diff --git a/backend/__tests__/unit/services/installableCatalogService.test.js b/backend/__tests__/unit/services/installableCatalogService.test.js index 2c36f8c50b..2db7b55ee4 100644 --- a/backend/__tests__/unit/services/installableCatalogService.test.js +++ b/backend/__tests__/unit/services/installableCatalogService.test.js @@ -40,6 +40,46 @@ const githubTool = () => ({ describe('installable catalog service', () => { beforeEach(() => jest.clearAllMocks()); + it('carries the flag beside the message, so the page can tell a reason from an exception', async () => { + mockInstallables([ + { installableId: 'telegram', name: 'Telegram', description: 'Telegram description' }, + { installableId: 'slack', name: 'Slack', description: 'Slack description' }, + ]); + InstallableInstallation.find.mockReturnValue(lean([ + { + _id: installationId, + installableId: 'telegram', + status: 'error', + errorMessage: "This connector's channel is gone. Retry to rebuild it.", + // The reconciler's own constant: a writer declared it readable. + errorMessageUserFacing: true, + targetId: userId, + installedBy: userId, + components: [], + }, + { + _id: '6a8f6dc7a1dccf2e02f31099', + installableId: 'slack', + status: 'error', + // `markProjectionFailure`'s raw exception: the same field, a different + // kind of string, and the flag is what separates them. + errorMessage: 'connect ECONNREFUSED 10.4.4.7:443', + targetId: userId, + installedBy: userId, + components: [], + }, + ])); + Integration.find.mockReturnValue(lean([])); + + const catalog = await catalogFor(userId); + const byId = Object.fromEntries(catalog.installables.map((entry) => [entry.installableId, entry.installation])); + + expect(byId.telegram.errorMessageUserFacing).toBe(true); + expect(byId.slack.errorMessageUserFacing).toBe(false); + // The message still travels — the page, not the mapper, decides what to show. + expect(byId.slack.errorMessage).toBe('connect ECONNREFUSED 10.4.4.7:443'); + }); + it('returns provider readiness and the caller parent without leaking private fields', async () => { mockInstallables([ { installableId: 'telegram', name: 'Telegram', description: 'Telegram description' }, @@ -49,7 +89,7 @@ describe('installable catalog service', () => { _id: installationId, installableId: 'telegram', status: 'error', - errorMessage: 'projection missing', + errorMessage: "This connector's channel is gone. Retry to rebuild it.", targetId: 'another-user', installedBy: 'another-user', claimId: 'private-claim', @@ -89,7 +129,10 @@ describe('installable catalog service', () => { available: true, installation: { status: 'error', - errorMessage: 'projection missing', + errorMessage: "This connector's channel is gone. Retry to rebuild it.", + // The mapper hands over the provenance with the message, so the page + // never has to guess from the shape of the string (TASK-131). + errorMessageUserFacing: false, boundPodId: podId, claimedAt: '2026-09-05T00:00:00.000Z', updatedAt: '2026-09-05T00:01:00.000Z', diff --git a/backend/__tests__/unit/services/installableInstallationService.test.js b/backend/__tests__/unit/services/installableInstallationService.test.js index 18a4373d46..fa0b61295e 100644 --- a/backend/__tests__/unit/services/installableInstallationService.test.js +++ b/backend/__tests__/unit/services/installableInstallationService.test.js @@ -213,6 +213,12 @@ describe('installable connector projection', () => { const failed = await InstallableInstallation.findOne({ installableId: 'telegram' }); const inactive = await Integration.findOne({ installationId: String(failed._id) }); expect(failed.status).toBe('error'); + // `markProjectionFailure` stores the projector's own exception text verbatim + // — stack-adjacent detail like `connect ECONNREFUSED :443` — so it is + // the writer that must say `false`. The catalog row renders this field only + // when the flag is true, and the row belongs to the installer, not to us + // (TASK-131, vera 73848). + expect(failed.errorMessageUserFacing).toBe(false); expect(inactive.isActive).toBe(false); expect(inactive.config.connectCode).toBeUndefined(); @@ -595,8 +601,20 @@ describe('installable connector projection', () => { const reconciled = await sweep(new Date()); expect(reconciled.errored).toBe(2); - expect((await InstallableInstallation.findById(installing._id)).status).toBe('error'); - expect((await InstallableInstallation.findById(activating._id)).status).toBe('error'); + const expiring = await InstallableInstallation.findById(installing._id); + const expired = await InstallableInstallation.findById(activating._id); + expect(expiring.status).toBe('error'); + expect(expired.status).toBe('error'); + // Both reasons are our own constants, written for the person reading the + // Connectors page, so the writer states that beside the message. Without it + // the catalog row falls back to the generic sentence and the operator loses + // the only sentence that says what to do (TASK-131). + expect(expiring.errorMessageUserFacing).toBe(true); + expect(expired.errorMessageUserFacing).toBe(true); + // The copy itself is pinned here: these two strings are the whole set of + // reasons the catalogue will ever print verbatim, so a writer that changes + // one has to change this line too (wren 73914, vera 73915). + expect(expiring.errorMessage).toBe('Setup was interrupted before it finished. Try again.'); }); it('makes a missing active projection a retriable parent error', async () => { @@ -608,7 +626,10 @@ describe('installable connector projection', () => { const parent = await InstallableInstallation.findById(installed.installation._id); expect(reconciled.staleComponents).toBe(1); expect(parent.status).toBe('error'); - expect(parent.errorMessage).toBe('projection missing'); + expect(parent.errorMessage).toBe("This connector's channel is gone. Retry to rebuild it."); + // The reconciler's own constant: a message written for a person, so it + // carries the flag the page requires before rendering (TASK-131). + expect(parent.errorMessageUserFacing).toBe(true); expect(parent.components.every((component) => component.status === 'stale')).toBe(true); const retried = await install({ installableId: 'telegram', installedBy: userId, podId }); diff --git a/backend/models/InstallableInstallation.ts b/backend/models/InstallableInstallation.ts index c069f1be4e..e02f27ad6b 100644 --- a/backend/models/InstallableInstallation.ts +++ b/backend/models/InstallableInstallation.ts @@ -135,6 +135,14 @@ export interface IInstallableInstallation extends Document { claimId?: string; claimedAt?: Date; errorMessage?: string; + /** + * Whether `errorMessage` was written for a person. Two kinds of string land + * in that field — the reconciler's human-authored constants, and + * `markProjectionFailure`'s raw exception message — and the Connectors page + * renders them through one branch, so only a flagged message is printed + * (vera 73848). Set together with the message by the builders below. + */ + errorMessageUserFacing?: boolean; staleSince?: Date; /** Canonical pause state; Integration.config.adminPause is its projection. */ adminPause?: IInstallationAdminPause; @@ -230,6 +238,7 @@ const InstallableInstallationSchema = new Schema( claimId: { type: String }, claimedAt: { type: Date }, errorMessage: { type: String }, + errorMessageUserFacing: { type: Boolean, default: false }, staleSince: { type: Date }, adminPause: { reason: { type: String }, diff --git a/backend/services/connectorDeliveryFailureService.ts b/backend/services/connectorDeliveryFailureService.ts index 60e5db09ec..56709c893a 100644 --- a/backend/services/connectorDeliveryFailureService.ts +++ b/backend/services/connectorDeliveryFailureService.ts @@ -115,6 +115,11 @@ interface FlipInput { * its only writer, and the page renders the message only when it is set; a * writer that does not set it falls back to the generic sentence instead of * printing our stack text in a connector row (vera 73848). + * + * Stated, not defaulted: a `$set` that omits this key leaves the row's stored + * value in place, so omitting the flag is only safe on a row that has never + * carried one. `externalFeedService` names `errorMessageUserFacing: false` for + * exactly that reason — absence would be a claim it could not make. */ const userFacingError = (reason: string) => ({ status: 'error', diff --git a/backend/services/externalFeedService.ts b/backend/services/externalFeedService.ts index cfff6c7b02..c25c42b5dc 100644 --- a/backend/services/externalFeedService.ts +++ b/backend/services/externalFeedService.ts @@ -435,11 +435,15 @@ async function syncExternalFeeds(): Promise { $set: { status: 'error', // Diagnostic, not copy. This is a provider's error text or a raw - // exception message, so `errorMessageUserFacing` stays unset and - // the Connectors page shows its generic sentence for these rows — - // the detail is kept here for whoever debugs the feed instead of - // being printed in someone's connector list (vera 73848). + // exception message, so the detail is kept here for whoever debugs + // the feed rather than printed in someone's connector list (vera + // 73848). The flag is stated instead of left out because a `$set` + // that omits a key inherits the row's stored value: omitting it is + // only safe on a row that has never carried one, and naming it + // `false` is what makes the page's generic sentence this row's + // outcome no matter what it carried before. errorMessage: detail, + errorMessageUserFacing: false, lastSync: new Date(), }, }); diff --git a/backend/services/installable/installableCatalogService.ts b/backend/services/installable/installableCatalogService.ts index 5b6d8dde32..649652ce1b 100644 --- a/backend/services/installable/installableCatalogService.ts +++ b/backend/services/installable/installableCatalogService.ts @@ -56,7 +56,15 @@ const publicInstallation = (installation: any): unknown => { })) : []; return { status: installation.status, - ...(installation.errorMessage ? { errorMessage: installation.errorMessage } : {}), + // The message travels with its provenance. The page prints it only when this + // is true, so a raw exception from `markProjectionFailure` reaches the + // client and is deliberately not rendered (TASK-131). + ...(installation.errorMessage + ? { + errorMessage: installation.errorMessage, + errorMessageUserFacing: installation.errorMessageUserFacing === true, + } + : {}), ...(installation.boundPodId ? { boundPodId: String(installation.boundPodId) } : {}), ...(installation.claimedAt ? { claimedAt: installation.claimedAt } : {}), ...(installation.updatedAt ? { updatedAt: installation.updatedAt } : {}), diff --git a/backend/services/installable/installableInstallationService.ts b/backend/services/installable/installableInstallationService.ts index c58632a6c6..cad45f52ee 100644 --- a/backend/services/installable/installableInstallationService.ts +++ b/backend/services/installable/installableInstallationService.ts @@ -321,6 +321,32 @@ const claimInstallation = async ( } }; +/** + * The two ways `InstallableInstallation.errorMessage` gets a value, each with + * its provenance attached. + * + * `errorMessage` alone cannot say the reason is fit to read. Three writers give + * it a value: `markProjectionFailure` (a raw exception — + * `connect ECONNREFUSED :443`), and the reconciler's two human-authored + * constants. The Connectors page renders every failed installation through one + * branch, so "the field has a value" is not the same fact as "the value was + * written for a person" (vera 73848). `errorMessageUserFacing` is that fact, and + * a message and its flag are written together here so that no writer can set + * half of the pair — the catalog row is a stranger's row, and printing our stack + * text in it is disclosure, not diagnosis. + */ +export const userFacingInstallationError = (reason: string) => ({ + status: 'error', + errorMessage: reason, + errorMessageUserFacing: true, +}); + +export const diagnosticInstallationError = (error: Error) => ({ + status: 'error', + errorMessage: error.message, + errorMessageUserFacing: false, +}); + const throwIfLockLost = (installation: IInstallableInstallation | null): IInstallableInstallation => { if (!installation) throw new InstallLockLostError(); return installation; @@ -336,8 +362,7 @@ const markProjectionFailure = async ( { _id: installation._id, status: 'installing', claimId }, { $set: { - status: 'error', - errorMessage: error.message, + ...diagnosticInstallationError(error), components, }, }, @@ -688,4 +713,8 @@ module.exports = { InstallableNotFoundError, install, uninstall, + // Named here as well as exported above: this file's CommonJS tail replaces the + // ES exports, so an `export const` on its own is invisible to every require(). + userFacingInstallationError, + diagnosticInstallationError, }; diff --git a/backend/services/installable/installableReconciler.ts b/backend/services/installable/installableReconciler.ts index e454b2b643..9d4d50d698 100644 --- a/backend/services/installable/installableReconciler.ts +++ b/backend/services/installable/installableReconciler.ts @@ -1,5 +1,5 @@ import type { IInstallableInstallation } from '../../models/InstallableInstallation'; -import { INSTALL_LOCK_TTL_MS } from './installableInstallationService'; +import { INSTALL_LOCK_TTL_MS, userFacingInstallationError } from './installableInstallationService'; // eslint-disable-next-line @typescript-eslint/no-require-imports, global-require const InstallableInstallation = require('../../models/InstallableInstallation'); @@ -58,7 +58,10 @@ const sweepStaleLocks = async (now: Date): Promise<{ completed: number; errored: const result = await InstallableInstallation.updateOne( { _id: installation._id, status: installation.status, claimId }, - { $set: { status: 'error', errorMessage: 'install lock expired' } }, + // The copy is the outcome the operator reads in the Connectors list, so + // it names the event and the next step and no internal one: a lock is our + // word, not theirs (wren 73914, vera 73915). + { $set: userFacingInstallationError('Setup was interrupted before it finished. Try again.') }, ); errored += result.modifiedCount || 0; } @@ -87,8 +90,10 @@ const sweepActiveInstallations = async (): Promise<{ staleComponents: number; cl { _id: installation._id, status: 'active' }, { $set: { - status: 'error', - errorMessage: 'projection missing', + // Distinct from the page's generic 'Setup didn't finish.' on purpose: the + // connector existed and its channel record vanished, which is a different + // event, and the evidence pair has to read as two outcomes (vera 73915). + ...userFacingInstallationError("This connector's channel is gone. Retry to rebuild it."), 'components.$[].status': 'stale', }, }, diff --git a/docs/design/evidence/task-131-catalog-error-line-1200-after.png b/docs/design/evidence/task-131-catalog-error-line-1200-after.png new file mode 100644 index 0000000000..bf0206fb7d Binary files /dev/null and b/docs/design/evidence/task-131-catalog-error-line-1200-after.png differ diff --git a/docs/design/evidence/task-131-catalog-error-line-1200-after.txt b/docs/design/evidence/task-131-catalog-error-line-1200-after.txt new file mode 100644 index 0000000000..bec4e95bc5 --- /dev/null +++ b/docs/design/evidence/task-131-catalog-error-line-1200-after.txt @@ -0,0 +1,36 @@ +EN +LO +Connectors + +Each channel gets one agent from a pod. It answers there in its own name; the rest of the team stays behind it. + +Slack +no pod · since 13m +This connector's channel is gone. Retry to rebuild it. +retry, or remove it +Retry +Telegram +no pod · since 13m +Setup didn’t finish. +retry, or remove it +Retry +Discord · WhatsApp +not yet +Not yet. Tell us which channel you need and we build it next. +Ask + +next step + +This connector's channel is gone. Retry to rebuild it. + +Retry +Remove +Tools +1 granted +All +Granted +Not yet +GitHub +kai routing check · granted 6d +granted to Kai routing check +localdev may use it \ No newline at end of file diff --git a/docs/design/evidence/task-131-catalog-error-line-1200-before.png b/docs/design/evidence/task-131-catalog-error-line-1200-before.png new file mode 100644 index 0000000000..2a27c91058 Binary files /dev/null and b/docs/design/evidence/task-131-catalog-error-line-1200-before.png differ diff --git a/docs/design/evidence/task-131-catalog-error-line-1200-before.txt b/docs/design/evidence/task-131-catalog-error-line-1200-before.txt new file mode 100644 index 0000000000..9989c09260 --- /dev/null +++ b/docs/design/evidence/task-131-catalog-error-line-1200-before.txt @@ -0,0 +1,36 @@ +EN +LO +Connectors + +Each channel gets one agent from a pod. It answers there in its own name; the rest of the team stays behind it. + +Slack +no pod · since 13m +This connector's channel is gone. Retry to rebuild it. +retry, or remove it +Retry +Telegram +no pod · since 13m +connect ECONNREFUSED 10.4.4.7:443 +retry, or remove it +Retry +Discord · WhatsApp +not yet +Not yet. Tell us which channel you need and we build it next. +Ask + +next step + +This connector's channel is gone. Retry to rebuild it. + +Retry +Remove +Tools +1 granted +All +Granted +Not yet +GitHub +kai routing check · granted 6d +granted to Kai routing check +localdev may use it \ No newline at end of file diff --git a/docs/design/evidence/task-131-catalog-error-line-390-after.png b/docs/design/evidence/task-131-catalog-error-line-390-after.png new file mode 100644 index 0000000000..c2491ac0a6 Binary files /dev/null and b/docs/design/evidence/task-131-catalog-error-line-390-after.png differ diff --git a/docs/design/evidence/task-131-catalog-error-line-390-after.txt b/docs/design/evidence/task-131-catalog-error-line-390-after.txt new file mode 100644 index 0000000000..38899df28c --- /dev/null +++ b/docs/design/evidence/task-131-catalog-error-line-390-after.txt @@ -0,0 +1,34 @@ +Connectors + +Each channel gets one agent from a pod. It answers there in its own name; the rest of the team stays behind it. + +Slack +no pod · since 14m +This connector's channel is gone. Retry to rebuild it. +retry, or remove it +Retry +Telegram +no pod · since 14m +Setup didn’t finish. +retry, or remove it +Retry +Discord · WhatsApp +not yet +Not yet. Tell us which channel you need and we build it next. +Ask + +next step + +This connector's channel is gone. Retry to rebuild it. + +Retry +Remove +Tools +1 granted +All +Granted +Not yet +GitHub +kai routing check · granted 6d · every write asks first +granted to Kai routing check +localdev may use it \ No newline at end of file diff --git a/docs/design/evidence/task-131-catalog-error-line-390-before.png b/docs/design/evidence/task-131-catalog-error-line-390-before.png new file mode 100644 index 0000000000..2b4b8feccf Binary files /dev/null and b/docs/design/evidence/task-131-catalog-error-line-390-before.png differ diff --git a/docs/design/evidence/task-131-catalog-error-line-390-before.txt b/docs/design/evidence/task-131-catalog-error-line-390-before.txt new file mode 100644 index 0000000000..e0dea1742d --- /dev/null +++ b/docs/design/evidence/task-131-catalog-error-line-390-before.txt @@ -0,0 +1,34 @@ +Connectors + +Each channel gets one agent from a pod. It answers there in its own name; the rest of the team stays behind it. + +Slack +no pod · since 14m +This connector's channel is gone. Retry to rebuild it. +retry, or remove it +Retry +Telegram +no pod · since 14m +connect ECONNREFUSED 10.4.4.7:443 +retry, or remove it +Retry +Discord · WhatsApp +not yet +Not yet. Tell us which channel you need and we build it next. +Ask + +next step + +This connector's channel is gone. Retry to rebuild it. + +Retry +Remove +Tools +1 granted +All +Granted +Not yet +GitHub +kai routing check · granted 6d · every write asks first +granted to Kai routing check +localdev may use it \ No newline at end of file diff --git a/docs/plans/connector-as-installable-app.md b/docs/plans/connector-as-installable-app.md index 6298850d50..79d1289b0f 100644 --- a/docs/plans/connector-as-installable-app.md +++ b/docs/plans/connector-as-installable-app.md @@ -307,7 +307,7 @@ the slash-command / external-webhook tracks; naming them here keeps the enum hon row marks the component `stale` (never re-creates silently — a stale connector must not mint a code nobody asked for). For every `uninstalled` installation, projections must be inactive. For every **`installing`** installation whose `claimedAt` is older than `INSTALL_LOCK_TTL_MS`, -the sweep sets `status: 'error'` with `errorMessage: 'install lock expired'` — fenced on the +the sweep sets `status: 'error'` with `errorMessage: 'Setup was interrupted before it finished. Try again.'` — fenced on the `claimId` it read, so it cannot race a takeover that happened between its read and its write — and the row becomes the ordinary retryable case, and the board-facing state stops lying about work in progress. For a stale **`activating`** installation the sweep must **look @@ -420,7 +420,7 @@ Unit (`backend/__tests__/unit/services/installable/`): 5. Non-member of the chosen pod → 403, nothing written. 6. Reconciler: a deleted Integration under an active installation marks the component `stale`, creates nothing. An `installing` row with `claimedAt` older than the TTL is swept to - `error` with `'install lock expired'`. A stale `activating` row whose Integration is still + `error` with `'Setup was interrupted before it finished. Try again.'`. A stale `activating` row whose Integration is still inactive is swept to `error`; a stale `activating` row whose Integration is already active with a code is **completed to `active`**, the code unchanged, `mintConnectCode` not called — never demoted, so no redeemable code ever sits under an `error` parent. diff --git a/docs/plans/d8-phase-2-gate-surface.md b/docs/plans/d8-phase-2-gate-surface.md index a7fee9e264..58ec2b4bc0 100644 --- a/docs/plans/d8-phase-2-gate-surface.md +++ b/docs/plans/d8-phase-2-gate-surface.md @@ -6,7 +6,7 @@ ## 1. Three facts the page cannot see today -1. **Parent state is invisible.** There is no `GET` on `/api/installables`. The page lists `Integration` rows from `/api/integrations/user/all` and infers the parent from `installationId`. A parent whose projection failed (`status: 'error'`, no Integration row), whose lock expired (`errorMessage: 'install lock expired'`), or whose Integration went missing (`components[].status: 'stale'`) shows **nothing** — and the next Connect returns a typed 409 the user cannot act on. The invisible-error class we closed inside the substrate is still open at the surface. +1. **Parent state is invisible.** There is no `GET` on `/api/installables`. The page lists `Integration` rows from `/api/integrations/user/all` and infers the parent from `installationId`. A parent whose projection failed (`status: 'error'`, no Integration row), whose lock expired (`errorMessage: 'Setup was interrupted before it finished. Try again.'`), or whose Integration went missing (`components[].status: 'stale'`) shows **nothing** — and the next Connect returns a typed 409 the user cannot act on. The invisible-error class we closed inside the substrate is still open at the surface. 2. **The page carries the provider list.** `ADD_PLATFORMS` in `V2ConnectorsPage.tsx` says Telegram and Slack are self-serve. The instance knows better: Slack is self-serve only if `slackOAuthService` and the `ConnectorSecret` key ring are configured, Telegram only if the bot token is. A self-hosted instance without Slack secrets shows an *Authorize in Slack* button that fails at `authorize-url` with a configuration error. Vera asked for the flag at 63278. 3. **One pod per connector.** `Integration.podId` is required and is the single pod the connector relays. ADR-025 D8 says the private chat is the user's attention surface for **every** pod they are in, gated per pod. Phase 1 wrote the picked pod as "the first gate row" and stopped. @@ -33,7 +33,7 @@ "available": true, "installation": { "status": "error", - "errorMessage": "install lock expired", + "errorMessage": "Setup was interrupted before it finished. Try again.", "boundPodId": "66f…", "claimedAt": "2026-09-04T20:01:12Z", "updatedAt": "2026-09-04T20:02:12Z", @@ -62,7 +62,7 @@ | `error` | any | hollow `#98a2b3` | the parent's `errorMessage` as an ink sentence, else "Setup didn't finish." | `retry, or remove it` | `since {rel}` | **Retry** (ink) → `POST …/install { podId: boundPodId }`. The service's claim filter already admits `status: 'error'`; the parent is reclaimed in place, no second row. **Remove** is the bordered secondary in the aside (§2 D4) → `DELETE`, which `claimUninstall` accepts from `error` | | `active`, Integration present | per [#1542 §2](connectors-page-signal-diff.md) | as there | as there | as there | as there | Manage / Show code / New code / Authorize in Slack / Confirm — unchanged | | `active`, Integration present, `podId` unset (the owner left the active pod; D3 prune) | live | `#e4e7ec` solid (idle) | **{chat title}** {kind} · not linked to a pod | `your messages have nowhere to go` | `added {rel}` | **Pick a pod** (ink) → selects the row and opens the aside's gate list (D4) | -| `active`, Integration missing or `isActive: false`, or any `components[].status: 'stale'` | — | hollow | The channel record is gone. | `retry rebuilds it` | `since {rel}` | **Retry** (ink) — same POST; `installAttempt` re-projects. The reconciler's stale-marking (`sweepActive`) is what makes this row honest; it should also set `status: 'error'`, `errorMessage: 'projection missing'` so the parent and the page agree (one-line change in `installableReconciler.ts`) | +| `active`, Integration missing or `isActive: false`, or any `components[].status: 'stale'` | — | hollow | The channel record is gone. | `retry rebuilds it` | `since {rel}` | **Retry** (ink) — same POST; `installAttempt` re-projects. The reconciler's stale-marking (`sweepActive`) is what makes this row honest; it should also set `status: 'error'`, `errorMessage: "This connector's channel is gone. Retry to rebuild it."` so the parent and the page agree (one-line change in `installableReconciler.ts`) | | `uninstalling`, within TTL | any | cobalt pulsing | Removing… | `waiting for the server` | `started {rel}` | none | | `uninstalling`, past TTL | any | cobalt pulsing | Removal is taking longer than it should. | `the server let go of it` | `started {rel}` | **Retry remove** (bordered) → `DELETE` again; `claimUninstall` re-claims a stale `uninstalling` | | `paused` (written only by the admin lever, §2 D3) | any | hollow | Paused by an administrator. {reason} | `ask your operator` | `paused {rel}` from `adminPause.at` | none for the owner — Resume is the admin's, so the owner is never shown a control that will 403 | @@ -101,7 +101,7 @@ The page renders the gate list **only when the row carries `config.gates`**; a P | PR | what | size | depends on | |---|---|---|---| -| A | catalog route + `readiness()` per manifest + install refusal `provider_not_configured` + reconciler `projection missing` | S | — | +| A | catalog route + `readiness()` per manifest + install refusal `provider_not_configured` + reconciler channel-gone reason | S | — | | B | page: rows keyed by catalog, D2 state table, Retry / Remove / Cancel wired, poll on the catalog | M | A. **Folds into Kai's Signal restyle (TASK-007)** if that PR is still open — it re-keys the rows anyway; two PRs that both rewrite the row grid is one too many | | C | D8 schema: `scope`, optional `podId`, `config.gates`, outbound inversion, inbound on `podId` + membership at receive with the chat reply, PATCH gate and `podId` writes, migration script, reconciler prune (gates and the active pod); the admin pause/resume verbs with `paused` in the partial index, `liveClaimStatuses` and `statusForClaim`, the two owner-verb refusals, and the two sweep branches | M–L | — (backend only; the page ignores `gates` until D) | | D | page: gate list in the aside, `active` tag + Make active, the *not linked* row, per-pod overrides, Remove at the foot | S | B, C | @@ -114,7 +114,7 @@ A and C are independent and can run in parallel. B before C is fine: the page sh 2. **Error row → Retry.** Force a projection failure (projector throws) → parent `error`, no Integration; catalog shows the error row; `POST install` with the same `podId` reclaims the **same** parent `_id` (count of parents for that user stays 1) and reaches `active`. 3. **Cancel is TTL-honest.** Parent stuck `installing` with `claimedAt` 30 s old → `DELETE` answers 409 `install_in_progress`; at 61 s → `DELETE` answers 202 `uninstalling` and the parent's `claimId` changes. The page shows no Cancel at 30 s and shows it at 61 s (the row's `claimedAt` drives it, no client clock trick — inject the clock). 4. **Remove from error.** Parent `error` → `DELETE` → `uninstalled`; the catalog entry returns to `installation: null`; the orphan-secret sweep revokes any `ConnectorSecret` the failed attempt left. -5. **Projection missing.** Delete a live Integration under an `active` parent → the 5-minute sweep marks the parent `error` `projection missing`; catalog shows the row; Retry re-projects with the same `installationId`. +5. **Projection missing.** Delete a live Integration under an `active` parent → the 5-minute sweep marks the parent `error` `This connector's channel is gone. Retry to rebuild it.`; catalog shows the row; Retry re-projects with the same `installationId`. 6. **Gates route outbound.** User in pods A and B, `gates[A].enabled: true`, `gates[B]` absent → an escalation in B reaches the channel **zero** times, in A once. Flip B on → once. User leaves A → the prune drops `gates[A]` within one sweep and outbound from A stops immediately (membership is checked at send). 6b. **Leaving the active pod is heard, not swallowed.** User in pods A and B, `podId: A`, `gates[A]` and `gates[B]` enabled; user leaves A. A bare message typed before the sweep → no pod post (membership fails at receive) and one reply in the chat (*This connector has no active pod. Choose one in Commonly first.*); the sweep unsets `gates[A]` **and** `podId` within five minutes; a bare message after the sweep gets the same reply, still no post; the page shows the *not linked to a pod* row with **Pick a pod**; `PATCH { podId: B }` from the owner → the next bare message posts in B; the same PATCH from the pod-B creator → 403; `PATCH { podId: C }` for a pod the owner is not in → 403. Switching `gates[B]` off while `podId: B` changes nothing inbound: the owner's next message still posts in B. 7. **Gate write is membership-gated.** `PATCH` with a `gates` key for a pod the caller is not a member of → 403, no write; other keys in the same body are also refused (all-or-nothing). diff --git a/frontend/src/v2/__tests__/V2ConnectorsPage.test.tsx b/frontend/src/v2/__tests__/V2ConnectorsPage.test.tsx index 8b4203ac34..13deb86493 100644 --- a/frontend/src/v2/__tests__/V2ConnectorsPage.test.tsx +++ b/frontend/src/v2/__tests__/V2ConnectorsPage.test.tsx @@ -82,6 +82,11 @@ const renderPage = () => render( , ); +// The reconciler's own constant — the reason a person reads when the channel +// record behind a connector is gone. It replaced the old 'projection missing' +// wording in the TASK-131 copy pass (wren 73914, vera 73915). +const REASON_CHANNEL_GONE = "This connector's channel is gone. Retry to rebuild it."; + describe('V2ConnectorsPage', () => { beforeEach(() => { jest.clearAllMocks(); @@ -530,6 +535,35 @@ describe('V2ConnectorsPage', () => { }); }; + it('TASK-131: a catalog failure prints the generic sentence, not the raw exception that wrote the row', async () => { + // The catalogue half reads `InstallableInstallation.errorMessage`, whose + // projection-failure writer stores `error.message` verbatim. Same rule as + // the pod-scoped half: a value in the field is not permission to render + // it, and the flag is absent on this fixture. + const raw = 'connect ECONNREFUSED 10.4.4.7:443'; + mockCatalog([ + entry({ installation: { status: 'error', errorMessage: raw } }), + ]); + renderPage(); + + expect((await screen.findAllByText('Setup didn’t finish.')).length).toBeGreaterThan(0); + expect(screen.queryByText(raw)).toBeNull(); + }); + + it('TASK-131: a catalog message written for a person still renders verbatim', async () => { + // The reconciler's own reasons are written for the person reading this row, + // and the flag the builder sets beside them is what keeps them readable once + // the generic sentence became the fallback. + const reason = REASON_CHANNEL_GONE; + mockCatalog([ + entry({ installation: { status: 'error', errorMessage: reason, errorMessageUserFacing: true } }), + ]); + renderPage(); + + expect((await screen.findAllByText(reason)).length).toBeGreaterThan(0); + expect(screen.queryByText('Setup didn’t finish.')).toBeNull(); + }); + // Tools plan (Sam's option A, two lists): a tool Installable shares the // catalogue response but belongs to the Tools page, never to this one. it('a tool Installable in the catalogue never renders as a channel row', async () => { @@ -680,13 +714,23 @@ describe('V2ConnectorsPage', () => { it('offers Retry on an error parent, posting the bound pod, and Remove in the aside', async () => { mockCatalog([entry({ - installation: { status: 'error', errorMessage: 'projection missing', boundPodId: 'p2', updatedAt: new Date().toISOString(), components: [] }, + // The reconciler's own reason, and since TASK-131 it travels with the flag + // that says so: the row's line renders a message only when a writer + // declared it was written for a person. + installation: { + status: 'error', + errorMessage: REASON_CHANNEL_GONE, + errorMessageUserFacing: true, + boundPodId: 'p2', + updatedAt: new Date().toISOString(), + components: [], + }, })]); axios.post.mockResolvedValue({ data: { status: 'installing' } }); axios.delete.mockResolvedValue({ data: { status: 'uninstalled' } }); renderPage(); - expect((await screen.findAllByText('projection missing')).length).toBeGreaterThan(0); + expect((await screen.findAllByText(REASON_CHANNEL_GONE)).length).toBeGreaterThan(0); expect(screen.getByText('retry, or remove it')).toBeInTheDocument(); fireEvent.click(screen.getAllByRole('button', { name: 'Retry' })[0]); await waitFor(() => expect(axios.post).toHaveBeenCalledWith( diff --git a/frontend/src/v2/components/V2ConnectorsPage.tsx b/frontend/src/v2/components/V2ConnectorsPage.tsx index bc9683dfb1..5d6736c133 100644 --- a/frontend/src/v2/components/V2ConnectorsPage.tsx +++ b/frontend/src/v2/components/V2ConnectorsPage.tsx @@ -60,7 +60,13 @@ interface Connector { interface CatalogInstallation { status: string; + // The same pair the pod-scoped half reads, and the same rule: the field + // holding a value is not permission to show it. A catalog failure is + // `markProjectionFailure`'s raw exception message unless a writer says + // otherwise, so the flag decides and the generic sentence is the fallback + // (TASK-131, vera 73848). errorMessage?: string; + errorMessageUserFacing?: boolean; boundPodId?: string; claimedAt?: string; updatedAt?: string; @@ -820,7 +826,9 @@ const V2ConnectorsPage: React.FC = () => { actionLabel: t('connectors.retry', { defaultValue: 'Retry' }), detail: t('connectors.retryOrRemove', { defaultValue: 'retry, or remove it' }), dot: 'empty', - line: installation.errorMessage || t('connectors.setupFailed', { defaultValue: 'Setup didn’t finish.' }), + line: installation.errorMessageUserFacing && installation.errorMessage + ? installation.errorMessage + : t('connectors.setupFailed', { defaultValue: 'Setup didn’t finish.' }), pulse: false, when: since, };