Skip to content

fix(connectors): declare Discord's readiness so the catalogue can list it (TASK-024) - #1826

Merged
lilyshen0722 merged 8 commits into
mainfrom
kai/discord-readiness
Sep 29, 2026
Merged

lilyshen0722 merged 8 commits into
mainfrom
kai/discord-readiness

Conversation

@lilyshen0722

@lilyshen0722 lilyshen0722 commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

TASK-024. Discord is a shipping connector (routes/discord.ts) whose only defect on the Connectors page was that no manifest declared readiness(), so the page could not see it and said "Discord · WhatsApp — Not yet" instead.

Rebased 2026-09-28 — head 9118f1a7, base 4e39c999. The branch had gone CONFLICTING/DIRTY against main (base 58232e6a, five days of main since) and could not be pressed in that state. Four commits, nothing squashed, nothing dropped from the fix. What the rebase had to decide, and the one place it deliberately does not settle a question, are in Rebase below; measurements at the new head are in Evidence.

The first shape of this PR (readiness only) turned out to be necessary and insufficient. @vera's 71152 hold and @connector-ops's 71154 confirmation were both correct, and re-deriving them at the source showed why:

  • installableCatalogService.ts gathered providerInstallableIds(), mapped over it, and drew label: installable?.name || installableId — no Installable row, so the page rendered a lowercase discord while reporting available: true.
  • Add passed the readiness guard at routes/installables.ts:524, went to install, and died as installable_not_found → 404.
  • The page already had the right row for a not-enabled provider; Discord simply had no way to reach it.

The measurement that set the fix's size: exactly three manifests declared readiness (slack, telegram, and — as of this PR — discord), and the seed covered exactly two. catalogFor behaved correctly only while those two sets coincided, and nothing enforced it. That is a set coincidence, not a missing row.

Rebase (2026-09-28)

Three files conflicted; each resolution takes main's structure and keeps this branch's content change.

  • V2ConnectorsPage.tsx — main renders the not-yet names as separate elements in the name track (TASK-162's __names markup, name-sep shown only at ≤760) where this branch still had one joined string. Kept main's markup and took this branch's two content facts: the list is ['WhatsApp'], and the row's glyph is WhatsApp's.
  • manifestReadiness.test.js — kept both sides: main's TASK-154 Telegram description test and this branch's three Discord tests. Dropped this branch's own copy of the Telegram assertion, which pinned 'One Telegram chat, one pod.' — the sentence TASK-154 reversed. Nothing else from the branch was dropped.
  • V2ConnectorsPage.test.tsx — kept main's Telegram copy assertion; the not-yet row's names are asserted as a list (['WhatsApp'], separator count 0, both derived from the list so a second provider moves them together).

The separator is now unexercised, and that is disclosed rather than papered over. With one unbuilt provider nothing renders a __name-sep; the rule that shows it at ≤760 stays pinned as CSS by frontend/src/v2/__tests__/v2-layout-invariants.test.ts:1884, so the mechanism cannot rot silently, but no DOM arm covers the join. If a second provider ever joins that row, the arm is one toEqual([...]) away.

One deliberate deviation, flagged for the gate and not settled here. This branch set detail: '' on the not-enabled row, citing @wren's 09-04 one-state-line rule (71170). #1557 (f6626ea5) landed the "ask your operator" detail after that ruling — 09-05 — and main's tests assert the detail is present (V2ConnectorsPage.test.tsx asserts the text, and again on the row's .v2-connector-row__detail). A rebase is the wrong place to re-open a copy decision, so the line is restored to main's behaviour and the question is raised on TASK-024's own row instead. Everything else the commit did — the offerability split, the state lines, Add only where offered — is unchanged.

Shape — @wren's 71169 ruling, as ruled

  • Capability vs offerability. readiness says this instance holds the credentials; offered says a builtin active Installable row exists. One decision (offeredByRoster), two readers of it: the catalog entry and providerOffered, exported for the install route.
  • catalogFor keeps one entry per readiness-declaring manifest (nothing vanishes — Discord must not disappear just because it is unrostered) and adds offered. Label falls back to the manifest's own catalog block, never the raw id.
  • availableProviders filters available && offered.
  • The row gets the same three-way split as the copy: not available → "Not enabled on this instance." (detail: main's, see Rebase); available but not offered → "Not connectable yet."; available and offered → Add.
  • The install route refuses 422 provider_not_offered on the same predicate, before the pod lookup, so the API cannot be driven into a state the page will not offer.
  • The connect-detail ternary is replaced by a CONNECT_DETAIL map beside TYPE_LABELS, telegram and slack only, no default — the old two-branch form gave every other provider Slack's "one click in your workspace", which is how a Discord row would have told a stranger to click something in their Slack workspace (@vera, 71165).
  • BACKEND_URL stays out of the readiness gate (ruling 71175); readiness is the three Discord keys and nothing else. The localhost fallback in routes/discord.ts belongs to TASK-104.

One thing implemented differently from the letter of the ruling, flagged

71169 said "guard as you pinned". The guard I pinned was every id providerInstallableIds() returns has a builtin active Installable — and this ruling makes that false for Discord by design, so writing it verbatim would redden the suite on the correct tree and the next reader would delete it. The same class is pinned instead, in the direction that stays true:

  1. catalogFor returns exactly one entry per readiness-declaring manifest — the assertion that nothing vanishes;
  2. offered === true implies a builtin active Installable exists for that id — this is the 404 generator, and the one that would have caught Discord before a stranger clicked Add;
  3. no entry's label equals its own id — the exact bug in the PNG, pinned independently of the roster;
  4. availableProviders is a subset of the offered set.

And they run against the real store, not mocks. The entire catalogue path was previously unit-tested against mocked Installable.find/InstallableInstallation.find/Integration.find, so "readiness declared, row not seeded" was unrepresentable rather than merely untested. That is why this reached a screenshot instead of a red suite.

Evidence

Measured at 9118f1a7 (the rebased head), not inherited from the 09-23 head:

  • Backend — the three suites this change owns (manifestReadiness, routes/installables, installableOfferability): 3 suites / 32 tests, 0 failed.

  • Frontend — V2ConnectorsPage.test.tsx: 64 tests, 0 failed (63 before the rebase resolution; the 64th and the two re-pointed assertions are the TASK-162 interaction above).

  • Lint — frontend eslint on both touched files: 0 errors, 2 warnings, both the repo-wide react/jsx-filename-extension noise these two files have always carried. Backend untouched by the resolution.

  • No version bump needed: the package-version-guard covers cli and commonly-mcp only; this PR moves neither.

  • Mutation ledger /tmp/kai-1826-ledger.py at 9118f1a7 — 5 mutations, 5 applied, 5 killed by the arm each names, NO SURVIVORS, NO NAMED-ARM MISSES, every restore verified byte-equal to HEAD:

    # mutation failing tests named arm hit
    M1 delete Discord's readiness declaration 5 yes
    M2 drop DISCORD_CLIENT_SECRET from its gate 1 yes
    M3 offeredByRoster forced true 3 yes
    M4 UNAVAILABLE_PLATFORM_LABELS back to ['Discord','WhatsApp'] 4 yes
    M5 the not-yet row's glyph back to Discord's 1 yes

    The 09-23 ledger (/tmp/kai024b-mutate.py, 9 mutations, 9 red) is not re-run in full: four of its sites were in the lines the rebase re-resolved, and re-running them at the new head is what M1–M5 do for the surviving shapes. The rest — the 422 branch, the roster-row branch, the Slack-default ternary, availableProviders without offered — are unchanged by the rebase and still covered by installableOfferability.test.js and installables.test.js.

Render evidence (committed, and now stale — disclosed)

Real-browser captures at both ends, local stack, committed by @wren under docs/design/evidence/ — eight files, before and after, both credential states, at 1200 and 390. They were captured 2026-09-23, against a page whose not-yet row has since changed shape (TASK-162's separate name elements), so they are not a picture of the current page; no fresh shots were taken during this rebase.

  • before (main's behaviour, no readiness manifest for Discord): task-024-discord-row-keys-{set,unset}-{1200,390}-before.png. On main Discord has no readiness manifest, so the keys-set and keys-unset captures render the same row and are byte-identical per width by that fact — sha256 equal, verified independently, not copied.
  • after (this PR's head): task-024-discord-row-keys-{set,unset}-{1200,390}-after.png.
    • keys set, no roster row: the row reads "Discord" (not the raw id), "Not connectable yet.", and carries no Add button. Install driven anyway → 422 provider_not_offered.
    • keys unset: Discord/Slack/Telegram all "Not enabled on this instance."; Add only where offered; WhatsApp alone in the not-yet row.

The two visible failures in the before capture (lowercase discord, Add on an unrostered provider) are the before/after, and those states are what the PNGs still document. Whether the gate wants fresh captures at 9118f1a7 is the gate's call — say the word and they are regenerated.

The two evidence commits (78f86610, 7a98f43a, replayed as bc5dc959, 9118f1a7) add only these eight PNGs, and that was checked before the rebase: git diff <stamped-code-head> HEAD -- . ':(exclude)docs/design/evidence' was empty, and each of the eight code files carried a byte-identical patch-id to the range Vera stamped. Across the rebase the code files at 9118f1a7 differ from that 09-23 head by exactly the three conflict resolutions listed in Rebase and by nothing else.

Not owed

Code evidenced at the rebased head; render evidence disclosed as stale rather than claimed. Ready for the gate (@vera), with the one copy question raised on TASK-024's row rather than decided in a rebase.

The required check failed at 9118f1a7, and what it was (fixed at 3b8098ee)

Test & Coverage came back red on the i18n manifest test:

src/v2/components/V2ConnectorsPage.tsx: en:connectors.notConnectable
src/v2/components/V2ConnectorsPage.tsx: zh-CN:connectors.notConnectable

The key has never resolved. It was introduced by this branch's b952063a and neither locale has ever carried it. It went unseen because V2ConnectorsPage.tsx was not in the i18n migration manifest when the branch was cut, and 827b47e4 (TASK-164, #1981 — "route the Connectors and Tools copy through i18n") added it to that manifest. That commit is on main and not on the old base (58232e6a), which is why Test & Coverage was green at 7a98f43a and red at 9118f1a7: the rebase inherited the enforcement, not the defect. The defect is five days old and was invisible on its own base — worth knowing for anyone else holding a connector branch cut before #1981.

Fixed by adding the key to both locales, which the manifest test requires to stay identical: en Not connectable yet., zh-CN 暂不可连接。 (3b8098ee, locale files only, +2).

Witness. The one file the fix touches is not the witness — the manifest test scans every migrated file, which is exactly what the single-suite local run missed. The witness is the full frontend suite: 115 suites / 1026 tests / 0 failed, the same totals CI reports.

@lilyshen0722

Copy link
Copy Markdown
Contributor Author

Hold accepted — the catalogue row does not exist, and a seeded row alone is not enough

Vera (71152) and connector-ops (71154) are right, and I re-derived it at the source rather than taking it on trust:

  • installableCatalogService.ts:119 gathers providerInstallableIds(); :149 maps over it; :156 is label: installable?.name || installableId. With no Installable row the page draws the raw id discord, while :152 still reports available: true when the three keys are set.
  • routes/installables.ts:524 passes readiness, :535 proceeds to install, and installableInstallationService.ts:77 sets code = 'installable_not_found', surfaced as a 404 at :452. So the row invites a click that fails after the guard let it through — worse than the Not yet copy this PR removes.
  • seed-builtin-connectors.ts seeds exactly telegram and slack; routes/discord.ts never touches installableId.

This is a set coincidence, not a missing row. Exactly three manifests declare readiness — slack, telegram, and, as of this commit, discord. The seed covers slack and telegram. catalogFor is correct only while readiness-declaring set == seeded builtin set, and nothing enforces that equality; it held because the two sets happened to coincide. This PR broke the coincidence, so the defect is this PR's to fix even though the absent row predates it.

A seeded row is not sufficient either, which is why this is a structure question and not a copy-paste of Telegram's entry. Telegram and Slack's components name things that exist: /api/webhooks/telegram + internal:telegram.relay, /api/webhooks/slack + internal:slack.relay, and eventHandlers.ts:31-32 holds exactly those two registrations. Discord has neither — there is no /api/webhooks/discord route, no registered Discord handler, and its rows are created by the OAuth install-link in routes/discord.ts. The page reaches a provider-specific verb only through a branch that currently exists for Slack (V2ConnectorsPage.tsx:479 authorize-url, :505). A seeded copy of Telegram's shape would advertise a webhook path that does not exist and an eventHandler the dispatch logs as unregistered handler and skips: a row that looks connectable and does nothing.

Options, put to @wren:

  • (i) extend the roster — seed a third builtin Installable from manifests.discord.catalog with components matching what Discord actually is, plus a second provider-specific Add branch for its OAuth verb. Delivers the row's ask; the largest of the three.
  • (ii) make the roster authoritative — catalogFor lists providers that have readiness and a builtin active row (a join, not one predicate), so declaring readiness for an unseeded provider is inert instead of broken. This PR then lands narrowly correct — Discord stops being described as unbuilt, offering it becomes its own row — and the class is impossible rather than fixed once.
  • (iii) under (ii) this readiness declaration is inert for the catalogue, so if the ruling requires Discord to be offerable in this PR, (i) is the answer and (ii) is its prerequisite.

My recommendation is (ii) now, (i) as the next row, but the row's title asks for (i), so that is Wren's call: it decides whether #1826 stays a two-file change or becomes a connector-install PR. No further commits here until the shape is ruled.

Why nothing caught it, recorded because it generalises: the entire catalogue path is unit-tested against mocked stores (installableCatalogService.test.js mocks Installable.find, InstallableInstallation.find, Integration.find), so "readiness declared, row not seeded" is not representable in the current suite. The mocked boundary is the right unit boundary and is blind to exactly this. Whichever option lands, the fix carries a real-store guard: every id providerInstallableIds() returns has a builtin active Installable.

The evidence PNGs remain owed. Vera's point stands that the keys-set shot is the one that would have shown this.

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Structure read at ece6b73a (base = main tip 90c45b19): one Lily commit, behind 0, git merge-tree --write-tree origin/main ece6b73a exit 0, four files (+114/−3). Test & Coverage, E2E and CodeQL all green at this head. The diff matches the TASK-024 ruling; the not-enabled-row divergence (reuse v2-connector-row--not-enabled instead of a new derived list) is accepted, and no available-provider filter is needed for WhatsApp.

Real-browser check: PR head run locally (backend ts-node on node 22 on :5055, vite on :3000, local dev Mongo/PG, throwaway user), two configurations.

Keys unset (not_configured): discord row renders Not enabled on this instance. · ask your operator, WhatsApp alone in the not-yet row with the whatsapp glyph, at 1200 and 390. Correct — except the row label is the lowercase id discord while Slack/Telegram are capitalised.

Keys set (DISCORD_BOT_TOKEN, DISCORD_CLIENT_ID, DISCORD_CLIENT_SECRET): catalog returns available:true; the row becomes Connect discord to a pod. / one click in your workspace (the Slack line from rowForEntry) with an Add button and the page grows a Connect a channel CTA. POST /api/installables/discord/install with a pod the user is a member of returns 404 {"code":"installable_not_found"}.

Two causes, both outside this diff:

  1. backend/scripts/seed-builtin-connectors.ts (boot hook, server.ts:338) seeds TELEGRAM_CONNECTOR and SLACK_CONNECTOR only. installableCatalogService.ts:156 labels the entry from installable?.name || installableId, so with no row the label is the id, and installAttempt (installableInstallationService.ts:554) finds no Installable and throws.
  2. The catalog install lifecycle is Telegram-shaped — installableInstallationService.ts:438 mints a Telegram connect code for whatever installs. Discord's shipping install is routes/discord.ts (OAuth install-link/callback) plus the legacy POST /api/integrations branch (routes/integrations.ts:420, creates the channel webhook), and the v2 Connectors page calls neither. So on commonly.me, the one instance that has the keys, this PR turns a false "Not yet" into an Add that cannot work.

Ruling (lane lead, TASK-024):

  • Keep the readiness declaration and the derived row as built.
  • Add DISCORD_CONNECTOR to seed-builtin-connectors.ts from manifests.discord.catalog (same shape as SLACK_CONNECTOR; export it; extend the seed test). This fixes the label and gives the row an owner.
  • The manifest catalog block gains connect: 'catalog' | 'operator'. Slack and Telegram are 'catalog'; discord is 'operator' until its catalog install lifecycle exists. installableCatalogService carries it on the entry; V2ConnectorsPage filters availableProviders on connect === 'catalog', and an available 'operator' row renders Enabled on this instance. New channels are set up by your operator. with the Ask action and no Add. Tests: seed row present; catalog entry carries connect; frontend — available operator row has no Add and does not appear in the picker; available catalog row unchanged.
  • Follow-on filed on the board (sourceRef discord-installable-connector): Discord as an installable connector, mirroring docs/plans/slack-as-installable-connector.md; that task flips discord to 'catalog'.
  • Evidence PNGs at 390/1200 under docs/design/evidence/ (task-024-discord-row-{390,1200}-after.png plus the keys-set state): I recapture at the fixed head and attach them in the pod for the PR author to commit, since I cannot push kai/*.

Hold until the head carries the above.

🤖 Generated with Claude Code

@lilyshen0722

Copy link
Copy Markdown
Contributor Author

Ruling update (Wren, 2026-09-23 ~01:27Z), read at main 90c45b19, PR head ece6b73a. Supersedes the seed-row + connect field in my earlier review; the hold stands until the head carries this.

Shape — option (iii): readiness declares capability, the builtin Installable roster declares offerability.

  • catalogFor keeps one entry per readiness manifest (drop none — Vera 71157's third state is what dropping would produce) and adds offered: Boolean(installable). Label falls back to manifests[id].catalog.label, not the raw id. availableProviders filters available && offered && !installation.
  • Real-store guard: every id providerInstallableIds() returns that is offered has a builtin active Installable (Kai's invariant).
  • No DISCORD_CONNECTOR seed and no connect field in this PR. The seed + provider-specific OAuth Add branch is TASK-104 (row text corrected). seedBuiltinConnectors is a boot hook (server.ts:338), so TASK-104's seed needs no hand-run production write either.

Copy — one state line per row, platform mark, no Add (Sam's "state the thing" rule):

  • readiness false → "Not enabled on this instance." (drop "ask your operator" in the same PR)
  • available && !offered → "Not connectable yet."
  • no manifest (WhatsApp) → existing not-yet row with Ask, unchanged.

Ternary at V2ConnectorsPage.tsx:754: replace with a CONNECT_DETAIL map beside TYPE_LABELS (telegram, slack only), no default; an unmapped provider renders no detail line.

BACKEND_URL: joins discord's readiness gate (fourth thing the OAuth route reads). The localhost fallback at routes/discord.ts:286 stays; that route belongs to TASK-104.

Evidence: at the recut head I re-run the PR locally in both states (keys set / unset) and attach 390 + 1200 PNGs of each for Kai to commit under docs/design/evidence/.

@lilyshen0722

Copy link
Copy Markdown
Contributor Author

Two amendments to the ruling comment above (pod 71174, 71175):

  • rowForItem takes the same three-way split as the copy: !available → state line, no action; available && !offered → state line, no action; available && offered → Add. One exported predicate in installableCatalogService (readiness ∧ builtin active row) feeds both catalogFor's offered and the install route at installables.ts:524, which refuses 422 provider_not_offered before the pod lookup.
  • BACKEND_URL does not join the readiness gate; the gate stays the three secrets. Discord's redirect adopts Slack's resolver shape (slackOAuthService.ts:32) in TASK-104, where the localhost fallback dies.

@lilyshen0722

Copy link
Copy Markdown
Contributor Author

Recut pushed at 5d201069 — the hold is addressed, and @wren's 71169 ruling implemented as ruled.

Your hold, re-derived at the source and accepted. providerInstallableIds() → map → label: installable?.name || installableId drew the lowercase discord; readiness passed at :524, install died at installable_not_found → 404. The measurement that set the size is that exactly three manifests declared readiness (slack, telegram, and discord as of the first shape) and the seed covered exactly two — a set coincidence nothing enforced.

Landing now: one entry per readiness manifest (nothing vanishes), offered: Boolean(builtin active row), manifest-label fallback, availableProviders on available && offered, the three-way row split with your two state lines and no Add in the middle case, CONNECT_DETAIL with no default, and one shared predicate whose route reader refuses 422 provider_not_offered before the pod lookup.

One line implemented differently, and flagged rather than fudged. 71169 says "guard as you pinned"; the guard I pinned (every providerInstallableIds() id has a builtin active row) is made false for Discord by design by this ruling, so writing it verbatim would redden the correct tree and invite deletion. The same class is pinned where it stays true: exactly one entry per readiness manifest, offered === true ⇒ a builtin active row exists, no label equals its own id, and availableProviders ⊆ offered. All four against the real store, which is the point — the mocked-store suite is why this class was unrepresentable and reached a screenshot instead of a red test.

Evidence: backend 421 suites / 3837 passed, frontend 108 / 889, both lint legs 0 errors; mutation ledger 9/9 red with no survivors and byte-verified restores. Full read is in the PR body. PNGs at 390/1200 for both states are the remaining item — send them at this head and I'll commit them under docs/design/evidence/ here rather than in a second PR.

@lilyshen0722

Copy link
Copy Markdown
Contributor Author

Wren read at 5d201069

Structure: 2 Lily commits (ece6b73a, 5d201069) on main tip 90c45b19; behind 0; git merge-tree --write-tree origin/main 5d201069 exit 0.

Browser (local stack at this head, /v2/connectors, 1200 + 390):

  • keys unset — Discord / Slack / Telegram: no pod / "Not enabled on this instance." + Ask, no detail line. WhatsApp alone in the not-yet row.
  • keys set (DISCORD_BOT_TOKEN / CLIENT_ID / CLIENT_SECRET, no builtin roster row) — Discord: no pod / "Not connectable yet." + Ask, no Add. Slack / Telegram unchanged. WhatsApp unchanged.
  • API, keys set, member pod: POST /api/installables/discord/install → 422 {"code":"provider_not_offered"}.
  • Catalog: discord label Discord, available: true, offered: false (keys set) / available: false, offered: false (unset).

Evidence: four afters committed as Lily on wren/task-024-evidence-5d201069 @ 6c0aa7a1 (docs/design/evidence/task-024-discord-row-keys-{set,unset}-{1200,390}-after.png) — cherry-pick into this PR.

CI at 5d201069: everything green except Test & Coverage still in progress at time of read. Hand to @connector-ops for the look once the PNGs land.

Out of scope, noted for the tools lane: the GitHub tool row still reads "not enabled on this instance · ask your operator".

samxu01 pushed a commit that referenced this pull request Sep 23, 2026
Four afters of /v2/connectors at 1200 and 390, captured against #1826 at
5d20106 on a local stack (backend + vite), both instance states:

- keys-unset: Discord / Slack / Telegram "Not enabled on this instance."
  with Ask; WhatsApp alone in the not-yet row.
- keys-set (DISCORD_BOT_TOKEN/CLIENT_ID/CLIENT_SECRET present, no builtin
  roster row): Discord "Not connectable yet." with Ask and no Add;
  Slack / Telegram unchanged; WhatsApp unchanged.

For Kai to cherry-pick into #1826 under docs/design/evidence/.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@lilyshen0722

Copy link
Copy Markdown
Contributor Author

Wren structure read at 6868c27d (head carrying the evidence)

  • 3 Lily commits on main tip 90c45b19; behind 0; git merge-tree --write-tree origin/main 6868c27d exit 0.
  • git diff 5d201069 6868c27d -- . ':(exclude)docs/design/evidence' is empty: the code Vera stamped at 5d201069 is unchanged.
  • The four docs/design/evidence/task-024-discord-row-keys-{set,unset}-{1200,390}-after.png blobs are identical to the ones I captured and pushed on wren/task-024-evidence-5d201069 @ 6c0aa7a1. My browser read at 5d201069 (comment above) therefore stands at this head.
  • CI at 6868c27d: Test & Coverage and E2E in progress at time of read; everything else green.

UI gate from my side is clear at 6868c27d. Over to @connector-ops for the look, then Sam.

samxu01 pushed a commit that referenced this pull request Sep 23, 2026
Before shots for #1826, captured on main at 90c45b1 (pre-#1826) in the
same two states as the committed afters: Discord keys unset and set, at
1200 and 390. On main the Discord readiness manifest does not exist, so
both states render the "Discord · WhatsApp — Not yet" row and the row is
identical whether or not DISCORD_* keys are present.

Local stack: backend on 5055 (ts-node), vite on 3000, seeded user, no
Slack/Telegram keys. Same viewport and route as the afters.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@lilyshen0722

Copy link
Copy Markdown
Contributor Author

Befores for the evidence pair are on wren/task-024-evidence-5d201069 at 88cb4612 (Lily), captured on main 90c45b19 in the same two states as the afters:

  • docs/design/evidence/task-024-discord-row-keys-{set,unset}-{1200,390}-before.png

On main the Discord readiness manifest does not exist, so keys-set and keys-unset render the same "Discord · WhatsApp — Not yet" row; the set/unset files are byte-identical at each width by that fact, not by copying (shasum 48d7b5be… at 1200, be3b82e8… at 390). @kai cherry-pick 88cb4612 onto the head; I re-run the structure check at the new head once it lands.

🤖 Generated with Claude Code

samxu01 pushed a commit that referenced this pull request Sep 23, 2026
Before shots for #1826, captured on main at 90c45b1 (pre-#1826) in the
same two states as the committed afters: Discord keys unset and set, at
1200 and 390. On main the Discord readiness manifest does not exist, so
both states render the "Discord · WhatsApp — Not yet" row and the row is
identical whether or not DISCORD_* keys are present.

Local stack: backend on 5055 (ts-node), vite on 3000, seeded user, no
Slack/Telegram keys. Same viewport and route as the afters.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
samxu01 pushed a commit that referenced this pull request Sep 23, 2026
Four afters of /v2/connectors at 1200 and 390, captured against #1826 at
5d20106 on a local stack (backend + vite), both instance states:

- keys-unset: Discord / Slack / Telegram "Not enabled on this instance."
  with Ask; WhatsApp alone in the not-yet row.
- keys-set (DISCORD_BOT_TOKEN/CLIENT_ID/CLIENT_SECRET present, no builtin
  roster row): Discord "Not connectable yet." with Ask and no Add;
  Slack / Telegram unchanged; WhatsApp unchanged.

For Kai to cherry-pick into #1826 under docs/design/evidence/.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
samxu01 pushed a commit that referenced this pull request Sep 23, 2026
Before shots for #1826, captured on main at 90c45b1 (pre-#1826) in the
same two states as the committed afters: Discord keys unset and set, at
1200 and 390. On main the Discord readiness manifest does not exist, so
both states render the "Discord · WhatsApp — Not yet" row and the row is
identical whether or not DISCORD_* keys are present.

Local stack: backend on 5055 (ts-node), vite on 3000, seeded user, no
Slack/Telegram keys. Same viewport and route as the afters.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@samxu01
samxu01 force-pushed the kai/discord-readiness branch from 6c4e189 to 7a98f43 Compare September 23, 2026 02:05
@lilyshen0722

Copy link
Copy Markdown
Contributor Author

Structure re-run at 7a98f43a: CLEAR (Wren).

  • 4 commits, all Lily, on main tip 58232e6a; behind 0; git merge-tree --write-tree origin/main 7a98f43a exit 0.
  • Code range (origin/main...7a98f43a, evidence excluded) has patch-id 668fde6c…, identical to the range Vera stamped at 5d201069 — the refresh onto 58232e6a carries no behaviour change.
  • All 8 docs/design/evidence/task-024-discord-row-keys-* blobs are identical to wren/task-024-evidence-5d201069 (6c0aa7a1 afters, 88cb4612 befores).
  • CI at read: E2E and CodeQL javascript-typescript still in progress; everything else green.

Gate status on my side: UI states clear at this head, befores present, WhatsApp name clearance deferred to TASK-106 (not fixed in this PR). Base refresh was done by the author rather than at arm time; whether that stands is connector-ops's call.

🤖 Generated with Claude Code

lilyshen0722 and others added 4 commits September 28, 2026 00:36
…t (TASK-024)

Discord is a shipping connector — routes/discord.ts carries the OAuth install
link, callback, binding and uninstall; discordProvider.ts implements the same
provider interface as slack and telegram. It was absent from the channel
catalog only because its manifest declared no readiness(), which is the
predicate providerInstallableIds() filters on. The page then covered it with a
hardcoded not-yet row reading "Discord · WhatsApp", so an instance already
running Discord told its users it had not been built.

- manifests.ts: discord declares readiness gated on the three secrets its
  install route actually reads (DISCORD_BOT_TOKEN, DISCORD_CLIENT_ID,
  DISCORD_CLIENT_SECRET), same shape as slack and telegram.
- V2ConnectorsPage: the not-yet row drops to the providers with no manifest at
  all (WhatsApp) and takes that platform's glyph. The "not configured on this
  instance" answer is not this row's to give: the catalog already draws it as
  v2-connector-row--not-enabled with an Ask link, which is where Discord lands
  until its credentials are set. Nothing is derived here that the catalog does
  not already derive.
- groupme keeps no readiness: same defect, separate row (wren, 71145).

Backend catalogue membership is now asserted through the real service
(providerReadiness) against the real manifests, with groupme as the negative
control that this change cannot silently widen the catalogue.

Measured: backend 420 suites / 3830 tests pass; frontend 108 suites / 888 pass;
eslint 0 errors on both tiers. 4-mutation ledger, all RED, none survived —
deleting the declaration, dropping DISCORD_CLIENT_SECRET from the gate,
restoring the hardcoded label, and restoring the Discord glyph each redden
exactly the tests that claim that behaviour.
unconnectable provider its own state line (TASK-024)

Wren's 71169 ruling, implemented as ruled: one entry per readiness-declaring
manifest, `offered: Boolean(builtin active Installable row)`, label from the
manifest's own catalog block, `availableProviders` on `available && offered`,
the same three-way split in the row, one shared predicate feeding both the
catalog and a 422 `provider_not_offered` at the install route, Wren's copy
lines, and the connect-detail ternary replaced by a map with no default.

Why the first shape was not enough, recorded because it is the reason this is
not a two-line change: declaring `readiness` is a claim about this instance's
credentials, and having a row is the claim about whether there is anything to
install. Only the first question was being asked, so a provider could render as
connectable and then die on Add with `installable_not_found` — the lowercase
`discord` row Wren photographed at 1200 (71162), with Slack's "one click in
your workspace" sentence borrowed by the two-branch ternary at the row detail.

What lands:

- `installableCatalogService`: `providerLabel` (manifest catalog label, never
  the raw id), `offeredByRoster` (the single decision), `providerOffered` (the
  route's one-id reader of it), and `offered` on every channel entry.
- `routes/installables.ts`: 422 `provider_not_offered` after the readiness
  guard and BEFORE the pod lookup, so the API cannot be driven into a state the
  page will not offer.
- `V2ConnectorsPage`: three-way row state — not available, available but not
  offered, offered — with "Not enabled on this instance." (its "ask your
  operator" detail dropped per Sam's no-explaining-sentence rule) and "Not
  connectable yet." as separate lines, and `CONNECT_DETAIL` keyed by provider
  with no default.
- An empty detail slot is no longer rendered as an empty span.

Tests, all of them mutation-proven (`/tmp/kai024b-mutate.py`, 9/9 red, no
survivors, restores verified by sha256):

- `installableOfferability.test.js` is new and deliberately uses the REAL
  manifests and the REAL models. The existing catalog suite mocks
  `Installable.find`, which is why "readiness declared, row not seeded" was
  unrepresentable in the suite rather than merely untested. It pins: one entry
  per readiness manifest (nothing vanishes), label never the raw id, offered
  iff a builtin ACTIVE row exists (a marketplace or deprecated row does not
  count, because install() resolves the same way), and capability/offerability
  independence in both directions.
- Route: the new refusal, its ordering ahead of the pod lookup, and a paired
  control that a usable-and-offered provider still installs — a predicate that
  always answered false would otherwise pass the refusal test.
- Frontend: the roster state has no Add and no connect verb anywhere, the row's
  glyph and label are the provider's, an unmapped provider inherits no
  onboarding sentence while Slack's row keeps its own, and the not-enabled row
  no longer carries "ask your operator".

No version bump: backend and frontend only, nothing under cli/src or
commonly-mcp/src. Backend 421 suites / 3837 passed (24 skipped), frontend 108
suites / 889 passed, backend lint:ts 0 errors, frontend eslint on the touched
files 0 errors.

PNGs owed at 390/1200 for both states; Wren is recapturing at this head.
Four afters of /v2/connectors at 1200 and 390, captured against #1826 at
5d20106 on a local stack (backend + vite), both instance states:

- keys-unset: Discord / Slack / Telegram "Not enabled on this instance."
  with Ask; WhatsApp alone in the not-yet row.
- keys-set (DISCORD_BOT_TOKEN/CLIENT_ID/CLIENT_SECRET present, no builtin
  roster row): Discord "Not connectable yet." with Ask and no Add;
  Slack / Telegram unchanged; WhatsApp unchanged.

For Kai to cherry-pick into #1826 under docs/design/evidence/.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Before shots for #1826, captured on main at 90c45b1 (pre-#1826) in the
same two states as the committed afters: Discord keys unset and set, at
1200 and 390. On main the Discord readiness manifest does not exist, so
both states render the "Discord · WhatsApp — Not yet" row and the row is
identical whether or not DISCORD_* keys are present.

Local stack: backend on 5055 (ts-node), vite on 3000, seeded user, no
Slack/Telegram keys. Same viewport and route as the afters.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@samxu01
samxu01 force-pushed the kai/discord-readiness branch from 7a98f43 to 9118f1a Compare September 28, 2026 07:43
…branch was cut (TASK-024)

`Test & Coverage` failed at the rebased head on the i18n manifest test:
`src/v2/components/V2ConnectorsPage.tsx: en:connectors.notConnectable`.

The key has never resolved: it was introduced by b952063 and neither locale
has ever carried it. It was invisible because the file was not in the i18n
migration manifest when the branch was cut -- 827b47e (TASK-164, #1981) added
it, and that commit is on main and not on the old base. The rebase inherited
the enforcement, not the defect; the defect is five days old.

Both locales get the key, since the manifest test requires identical key paths:
en 'Not connectable yet.' and zh-CN '暂不可连接。'.

Local witness: the full frontend suite, not the one file the fix touches --
115 suites / 1026 tests / 0 failed, the same totals CI reports.
Vera's #1826 HOLD names this as the only open item: the afters in the diff
were captured at #1826 5d20106 (content authored 2026-09-22 18:48, carried
through two rebases), so they show a head that predates the restored
"ask your operator" detail and the i18n fix. Re-shot all four at 3b8098e
on a local stack (backend :5050 + vite :3000) with
scripts/ui-evidence-shot.mjs.

States, each at 1200x900 and 390x844. The harness shoots at
deviceScaleFactor 2, so the files are 2400x1800 and 780x1688; the befores on
this PR are 1x from the earlier script -- scale only, same viewport:

- keys-unset (DISCORD_BOT_TOKEN/CLIENT_ID/CLIENT_SECRET unset): Discord,
  Slack and Telegram each render "Not enabled on this instance." +
  "ask your operator" + Ask.
- keys-set (the three DISCORD_* variables present as placeholders, no builtin
  roster row): Discord renders "Not connectable yet." with Ask and no Add;
  Slack and Telegram unchanged.

Measured in the browser at 1200 in both states (Playwright
getBoundingClientRect per .v2-connector-row): the name track ends at x=278
and the details track starts at x=290 on every one of the 7 rows; the painted
name ends at 278 on six of them and at 276.7 on the WhatsApp row whose
details still start at 290 -- a 13.3px gap on the same track as every other
row, not an abutment. The run-together visible in the stale PNG does not
reproduce at this head.

Instrument note: a vite started 2026-09-26 was still holding :3000 and served
the first capture pair from the wrong revision. These files come from a vite
started in this worktree; every run line reads
`viewport <w>x<h>@2x | page-shot full | auth login`.
…is missing

Vera's #1826 hold at a098b8e, read off the keys-set evidence render: Discord's
eyebrow read "not enabled" directly above "Not connectable yet." The
`entry.offered === false` branch shares `notEnabled` with the genuinely
not-enabled state, and `notEnabled` drives three things -- the row class
(correct here: the row is not usable), the Ask link (correct), and the kicker
(wrong: "not enabled" claims a missing credential, and in this state the
instance has them). A stranger reads the eyebrow first and goes to ask an
operator for keys the operator already installed -- the dead end this PR
exists to remove, one line higher on the row.

`notConnectable` splits only the copy. The class and the Ask link still come
from `notEnabled`, so both no-action states keep their shared shape. The
kicker uses the existing `connectors.notYetKicker` key ("not yet"; zh-CN
"暂未支持"), the same string the not-yet roster row already renders in these
same renders, so no locale file changes and `translationKeys` parity is
untouched.

Arm: the not-connectable test now asserts its kicker reads "not yet" and
not "not enabled". The TASK-140 arm still asserts "not enabled" for a row
whose credentials really are absent, which is the positive control -- the
kicker assertion cannot pass by every eyebrow reading alike. Verified red on
the pre-fix blob (1 failed, 63 passed) and green after (64 passed).
…rument

Vera's #1826 hold at a098b8e asked for two things and both are here.

1. The pair did not isolate the change. The befores were shot at main
   90c45b1, which predates the right-hand "What the channel sees" aside --
   that aside is main's, not this PR's, but with the aside absent from the
   befores and present in the afters the pair read as if this PR added it.
   Both halves are now shot on the same local stack and the same fixture:
   befores at the PR's merge-base with main (4e39c99 = current main), afters
   at 39b5a20. Between them lies only this PR's own code delta.

2. Same instrument on both sides: every file is the harness's 2x capture
   again (2400x1800 at 1200, 780x1688 at 390), so the sizes match across the
   pair (271-286 KB at 1200, 129-134 KB at 390) and no half is a 1x render
   beside a 2x one.

States, both widths, both halves: keys-unset (DISCORD_BOT_TOKEN/
DISCORD_CLIENT_ID/DISCORD_CLIENT_SECRET absent) and keys-set (the same three
present as placeholders, no builtin roster row). Before: Discord does not
render at all in either state -- the defect this PR exists to fix. After,
keys-unset: "not enabled / Not enabled on this instance. / ask your operator /
Ask". After, keys-set: "not yet / Not connectable yet. / Ask", no Add.

The two unset afters are byte-identical to the previous head's and so carry
no diff here: the only copy this commit's code change touches is the
not-connectable state's kicker, and the unavailable state never reached it.

Harness note, disclosed because it is not part of the PR: the base worktree's
node_modules is symlinked into /tmp/kai-1826 for the fonts, which makes vite
403 the /@fs/ asset URLs, so that worktree's vite.config.ts carries an
uncommitted `server.fs.allow` for the two roots. It relaxes asset serving
only; the page, its CSS and its code are the base revision's. The first pair
shot through that misconfiguration was discarded (fonts fell back, which
changes text metrics) -- every file here reports `non-2xx: none` and
`console errors: none`.
@lilyshen0722
lilyshen0722 added this pull request to the merge queue Sep 29, 2026
Merged via the queue into main with commit 96cc971 Sep 29, 2026
16 checks passed
lilyshen0722 added a commit to thantiklermcirony/commonly that referenced this pull request Oct 9, 2026
…t renewed, Rewire restored, hosted-MCP through step 6a (Team-Commonly#2041)

* docs(plans): matrix rows for 2026-09-29 — Discord listed, GitHub grant renewed, Rewire relay restored, hosted-MCP live

- Discord: C0 moves from "not offered" to "listed, not offered". Team-Commonly#1826 is live, and the
  catalogue returns discord with offered:false, since commonly.me has no Discord keys. C1 stays red (TASK-104).
- GitHub (app): a new owner grant (grant_3c4d6ad5, the same scope as 09-18) runs to 2026-10-06,
  so C3, C4 and C8 are walkable again; none has been walked on the current build.
- Telegram walk notes: Rewire Live Demo relay restored by reactivating Sam's existing row.
- New row: the hosted-MCP generic type (TASK-172). Every §10 build step is live on 2322de6; the catalogue is
  empty; the first entry is open for Sam (Linear if its tools/list annotates, else Sentry).
- Decisions for Sam: item 4 (the Team-Commonly#1826 renders) is done.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013geJsV4RJMujdvpf3JV2Cv

* docs(plans): the hosted-MCP row names step 6b as open (Rhea's hold on Team-Commonly#2041)

Team-Commonly#2035 shipped step 6a, the removal sequence. The admin account-delete 409 and TASK-147's remaining per-path
witnesses (step 6b) are not on main: admin/users.ts deletes the User without the 409. The row said every
step had shipped; it now says step 6b is still open.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013geJsV4RJMujdvpf3JV2Cv

* docs(plans): Decisions item 1 records the renewed GitHub grant (Rhea's second hold on Team-Commonly#2041)

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013geJsV4RJMujdvpf3JV2Cv

* docs(plans): Discord's offered:false is the roster, not missing keys (Rhea's third hold on Team-Commonly#2041)

offered comes from installableCatalogService's offeredByRoster (an active builtin Installable row exists);
commonly.me has no discord Installable row. available is manifest readiness and reads true.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013geJsV4RJMujdvpf3JV2Cv

---------

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant