diff --git a/changelog.d/2799-connectors-app-secret-field.fixed.md b/changelog.d/2799-connectors-app-secret-field.fixed.md new file mode 100644 index 000000000..b73b2c5d1 --- /dev/null +++ b/changelog.d/2799-connectors-app-secret-field.fixed.md @@ -0,0 +1 @@ +Feishu's app mode drew the App Secret as a bare `.bt-secret-input` — the masked field the Secrets page only ever places inside a bordered `.bt-q-field` wrapper — so it showed as a grey `Paste or type the key` line under the App ID box with no box and no label, and the form read as if only the App ID could be filled in. The connector form now gives every masked credential that same wrapper, so app mode shows an App Secret box beside the App ID box and the webhook field gets the border it was missing too ([#2799](https://github.com/ThinkFlowLab/FlyBot/issues/2799)). diff --git a/core/client.js b/core/client.js index 2d131c7ae..9d7b9b452 100644 --- a/core/client.js +++ b/core/client.js @@ -18645,7 +18645,7 @@ function statusText2(connector) { } function CredentialField({ credential, value, onChange, onEnter, autoFocus }) { const label = t(credential.label); - if (credential.masked) return (0, import_react39.createElement)(MaskedInput, { value, onChange, onEnter, label, autoFocus }); + if (credential.masked) return (0, import_react39.createElement)("label", { className: "bt-q-field bt-secret-field" }, (0, import_react39.createElement)(MaskedInput, { value, onChange, onEnter, label, autoFocus })); return (0, import_react39.createElement)( "label", { className: "bt-q-field" }, diff --git a/core/src/client/connectors.js b/core/src/client/connectors.js index b9cf2a304..b253bc5a2 100644 --- a/core/src/client/connectors.js +++ b/core/src/client/connectors.js @@ -52,10 +52,13 @@ function statusText(connector) { } // One credential of the form (#1266): the definition says which fields a mode needs and whether a -// field is masked. A masked one is the same field the Secrets page uses, so it never comes back. +// field is masked. A masked one is the same field the Secrets page uses, so it never comes back; it +// is also drawn like the Secrets page draws it, inside the bordered `.bt-q-field` wrapper its bare +// `.bt-secret-input` needs, or the field shows as a placeholder line and the form reads as the +// unmasked fields only (#2799). function CredentialField({ credential, value, onChange, onEnter, autoFocus }) { const label = t(credential.label) - if (credential.masked) return h(MaskedInput, { value, onChange, onEnter, label, autoFocus }) + if (credential.masked) return h('label', { className: 'bt-q-field bt-secret-field' }, h(MaskedInput, { value, onChange, onEnter, label, autoFocus })) return h('label', { className: 'bt-q-field' }, h('input', { className: 'bt-input', type: 'text', value, 'aria-label': label, placeholder: label, spellCheck: false, autoFocus, diff --git a/core/test/connectors-app-secret-field.test.js b/core/test/connectors-app-secret-field.test.js new file mode 100644 index 000000000..0f32aea40 --- /dev/null +++ b/core/test/connectors-app-secret-field.test.js @@ -0,0 +1,96 @@ +// #2799: in Feishu's app mode the App Secret is a masked credential. The masked input is the Secrets +// page's bare `.bt-secret-input` — no border, no background — because it only reads as a field +// inside a `.bt-q-field` wrapper. The connector form drew it without one, so the App Secret appeared +// as a grey placeholder line under the App ID box and the form looked like "App ID only". +import assert from 'node:assert/strict' +import { test } from 'node:test' +import { fileURLToPath } from 'node:url' +import { runInNewContext } from 'node:vm' +import { build } from 'esbuild' +import { FEISHU_MODES } from '../src/host/connectors.js' + +// The client bundle is built once, as the Settings page loads it, and run with a small React stand-in +// whose hook state is fed per render (the same shape core/test/feishu.test.js uses). +const { outputFiles } = await build({ + stdin: { + contents: "export { ConnectorsPage } from './src/client/connectors.js'", + resolveDir: fileURLToPath(new URL('..', import.meta.url)), + }, + bundle: true, format: 'cjs', platform: 'browser', write: false, logLevel: 'silent', + external: ['react', 'react-dom', '@deepseek-ai/dsh-client-ui-primitives'], +}) +const h = (type, props, ...children) => ({ type, props: { ...props, children } }) +let states = [] +const react = { + createElement: h, Fragment: Symbol('F'), useEffect() {}, useLayoutEffect() {}, + useState: v => [states.length > 0 ? states.shift() : (typeof v === 'function' ? v() : v), () => {}], + useMemo: f => f(), useRef: current => ({ current }), useCallback: f => f, useSyncExternalStore: (_s, get) => get(), +} +const browser = { exports: {} } +runInNewContext(outputFiles[0].text, { + module: browser, exports: browser.exports, document: { hidden: false }, navigator: { language: 'en' }, localStorage: { getItem: () => null, setItem() {} }, + window: { matchMedia: () => ({ matches: false }) }, setTimeout, clearTimeout, + require: (name) => { + if (name === 'react') return react + if (name === 'react-dom') return { createPortal: node => node } + if (name === '@deepseek-ai/dsh-client-ui-primitives') return {} + throw new Error(`Unexpected platform module ${name}`) + }, +}) +const { ConnectorsPage } = browser.exports + +const expand = (node) => { + if (node === null || node === undefined || typeof node === 'boolean') return null + if (Array.isArray(node)) return node.map(expand) + if (typeof node !== 'object') return node + if (typeof node.type === 'function') return expand(node.type(node.props)) + return { ...node, props: { ...node.props, children: expand(node.props.children) } } +} +const all = (node, out = []) => { if (node && typeof node === 'object') { if (Array.isArray(node)) node.forEach(child => all(child, out)); else { out.push(node); all(node.props?.children, out) } } return out } +const text = node => (node === null || node === undefined ? '' : Array.isArray(node) ? node.map(text).join('') : typeof node === 'object' ? text(node.props.children) : String(node)) +const holds = (node, target) => node === target || (node && typeof node === 'object' && (Array.isArray(node) ? node.some(child => holds(child, target)) : holds(node.props?.children, target))) +const classes = node => String(node?.props?.className ?? '').split(/\s+/).filter(Boolean) + +const actions = { connectorConnect: async () => {}, connectorDisconnect: async () => {}, connectorRetry: async () => {}, connectorSend: async () => {} } +const FEISHU_OFF = { id: 'feishu', name: 'Feishu', state: 'off', kind: 'channel', account: null, mode: null, modes: FEISHU_MODES, allowed: [] } + +// ConnectorRow's hook order: form, bots, token, apps, picking, busy, error, message, copyState, mode, +// values. Everything after `mode` is left to its initializer. +const renderForm = (mode, values = {}) => { + states = [true, false, '', [], false, false, null, '', 'idle', mode, values] + const tree = expand(h(ConnectorsPage, { roster: { connectors: [{ ...FEISHU_OFF, mode }], bots: [], self: { memberId: 'me' } }, actions })) + states = [] + return tree +} + +const fieldWrapperOf = (tree, input) => all(tree).find(node => classes(node).includes('bt-q-field') && holds(node, input)) +const connectButton = tree => all(tree).find(node => node.type === 'button' && text(node) === 'Connect') + +test('#2799: app mode draws the masked App Secret inside the same bordered field wrapper as the App ID', () => { + const tree = renderForm('app') + const inputs = all(tree).filter(node => node.type === 'input') + assert.deepEqual(inputs.map(node => node.props['aria-label']), ['App ID', 'App Secret'], 'app mode asks for both credentials') + + const appId = inputs[0] + const secret = inputs[1] + const idWrapper = fieldWrapperOf(tree, appId) + const secretWrapper = fieldWrapperOf(tree, secret) + assert.ok(idWrapper, 'the App ID sits in a bordered .bt-q-field, as it always has') + assert.ok(secretWrapper, 'the App Secret field has no bordered wrapper: it renders as a bare .bt-secret-input, so the form reads as App ID only') + assert.deepEqual(classes(secretWrapper), ['bt-q-field', 'bt-secret-field'], 'the App Secret uses the same wrapper the Secrets page puts around MaskedInput') + assert.equal(secret.props.placeholder, 'Paste or type the key') + assert.equal(text(secretWrapper), '', 'the label stays the aria-label the field already carries, not a second visible row') +}) + +test('#2799: the webhook mode is unchanged — one masked field, still wrapped', () => { + const tree = renderForm('webhook') + const inputs = all(tree).filter(node => node.type === 'input') + assert.deepEqual(inputs.map(node => node.props['aria-label']), ['Webhook URL'], 'webhook mode still asks for one credential') + assert.ok(fieldWrapperOf(tree, inputs[0]), 'and that field keeps its bordered wrapper') +}) + +test('#2799: Connect stays disabled until both app fields have text', () => { + assert.equal(connectButton(renderForm('app', { app_id: 'cli_aa78608ba6f9dbb4' })).props.disabled, true, 'the App ID alone is not enough') + assert.equal(connectButton(renderForm('app', { app_secret: 'app-secret-value' })).props.disabled, true, 'nor is the App Secret alone') + assert.equal(connectButton(renderForm('app', { app_id: 'cli_aa78608ba6f9dbb4', app_secret: 'app-secret-value' })).props.disabled, false, 'both fields filled enable Connect') +}) diff --git a/docs/subsystems/core-client.md b/docs/subsystems/core-client.md index 668d2912a..bc31fd1df 100644 --- a/docs/subsystems/core-client.md +++ b/docs/subsystems/core-client.md @@ -274,7 +274,7 @@ Settings → Connectors (`src/client/connectors.js`) lists one row per connector The Feishu row (#1263) is a **sender** (`kind: 'sender'` from the host, not an id the client knows): the input is labelled **Feishu webhook URL** and its link goes to Feishu's custom-bot page, never to GitHub's token page. Because a sender has no tools there is no **Choose Bots** button and no *On for N Bots* line, and the Bot-settings **Connectors** card skips it (`connectedConnectors` drops `kind === 'sender'`), so no switch is offered that would change nothing. Once it is ready the row carries a text field with **Send**, which is the only caller of `connector-send`; a refused webhook shows **Could not connect: …** with **Try again** beside it. -The same row is the whole act of connecting Feishu (#1266): it draws one row, not two, and the form has a **mode** — **Custom bot webhook** (one masked URL, outbound only, exactly phase 1) or **App (long connection)** (App ID + App Secret, one field per `credential` the definition declares, the App ID not masked because it is not a secret). Connect submits `{ mode, credentials }` through `connectorConnect`, which passes an object body straight to `connector-connect` and keeps the older single `token` body for every other row; the status line names the mode that is live, and **Disconnect** clears both. With `kind: 'channel'` the row offers the same per-Bot **Choose Bots** switch GitHub and Composio have (`connectedConnectors` includes a channel, so `connector-bots.js` lists it; off by default, never for a Bot shared with someone else) — a webhook-only row still shows no switch. App mode also renders **允许的飞书用户**: the refused senders the host reports, each with **Allow** / **Block** (`connector-allow` / `connector-block`), plus a paste-an-`open_id` field for the case where nobody has messaged yet; nobody is allowed until the owner adds them, so connecting the app never lets a stranger spend the owner's tokens. +The same row is the whole act of connecting Feishu (#1266): it draws one row, not two, and the form has a **mode** — **Custom bot webhook** (one masked URL, outbound only, exactly phase 1) or **App (long connection)** (App ID + App Secret, one field per `credential` the definition declares, the App ID not masked because it is not a secret). Connect submits `{ mode, credentials }` through `connectorConnect`, which passes an object body straight to `connector-connect` and keeps the older single `token` body for every other row; the status line names the mode that is live, and **Disconnect** clears both. A masked credential is drawn the way the Secrets page draws one — inside the bordered `.bt-q-field` wrapper its bare `.bt-secret-input` needs — so app mode shows an App Secret box beside the App ID box rather than a grey placeholder line that read as App ID only (#2799). With `kind: 'channel'` the row offers the same per-Bot **Choose Bots** switch GitHub and Composio have (`connectedConnectors` includes a channel, so `connector-bots.js` lists it; off by default, never for a Bot shared with someone else) — a webhook-only row still shows no switch. App mode also renders **允许的飞书用户**: the refused senders the host reports, each with **Allow** / **Block** (`connector-allow` / `connector-block`), plus a paste-an-`open_id` field for the case where nobody has messaged yet; nobody is allowed until the owner adds them, so connecting the app never lets a stranger spend the owner's tokens. A row in the `error` state offers **Copy error** beside **Try again** (#2027): it hands the connector's stored `error` — up to the 2,000-character host window from [core-host](core-host.md) — to the shared `copiedToClipboard()`, reads **Copied** only when the write landed and **Could not copy the error** when the page has no clipboard or the write was refused, and is drawn only while the connector is in that state, so a `ready`/`off` row's actions are unchanged. Tests: `core/test/connectors-page.test.js`. diff --git a/patches/ds-bot/0001-core-fixes.patch b/patches/ds-bot/0001-core-fixes.patch index 9f111d567..fc939f531 100644 --- a/patches/ds-bot/0001-core-fixes.patch +++ b/patches/ds-bot/0001-core-fixes.patch @@ -229,7 +229,7 @@ diff --git a/src/client/browser-pane.js b/src/client/browser-pane.js diff --git a/src/client/connectors.js b/src/client/connectors.js --- a/src/client/connectors.js +++ b/src/client/connectors.js -@@ -1,41 +1,230 @@ +@@ -1,41 +1,233 @@ -import { createElement as h, useRef, useState } from 'react' +import { createElement as h, useCallback, useEffect, useRef, useState } from 'react' import { useMorph } from './motion.js' @@ -288,10 +288,13 @@ diff --git a/src/client/connectors.js b/src/client/connectors.js -function ConnectorRow({ connector, actions }) { +// One credential of the form (#1266): the definition says which fields a mode needs and whether a -+// field is masked. A masked one is the same field the Secrets page uses, so it never comes back. ++// field is masked. A masked one is the same field the Secrets page uses, so it never comes back; it ++// is also drawn like the Secrets page draws it, inside the bordered `.bt-q-field` wrapper its bare ++// `.bt-secret-input` needs, or the field shows as a placeholder line and the form reads as the ++// unmasked fields only (#2799). +function CredentialField({ credential, value, onChange, onEnter, autoFocus }) { + const label = t(credential.label) -+ if (credential.masked) return h(MaskedInput, { value, onChange, onEnter, label, autoFocus }) ++ if (credential.masked) return h('label', { className: 'bt-q-field bt-secret-field' }, h(MaskedInput, { value, onChange, onEnter, label, autoFocus })) + return h('label', { className: 'bt-q-field' }, + h('input', { + className: 'bt-input', type: 'text', value, 'aria-label': label, placeholder: label, spellCheck: false, autoFocus, diff --git a/patches/ds-bot/0008-client-bundle.patch b/patches/ds-bot/0008-client-bundle.patch index 5995a1606..b1aceda66 100644 --- a/patches/ds-bot/0008-client-bundle.patch +++ b/patches/ds-bot/0008-client-bundle.patch @@ -22651,7 +22651,7 @@ diff --git a/client.js b/client.js - const [error, setError] = (0, import_react25.useState)(null); +function CredentialField({ credential, value, onChange, onEnter, autoFocus }) { + const label = t(credential.label); -+ if (credential.masked) return (0, import_react39.createElement)(MaskedInput, { value, onChange, onEnter, label, autoFocus }); ++ if (credential.masked) return (0, import_react39.createElement)("label", { className: "bt-q-field bt-secret-field" }, (0, import_react39.createElement)(MaskedInput, { value, onChange, onEnter, label, autoFocus })); + return (0, import_react39.createElement)( + "label", + { className: "bt-q-field" }, diff --git a/patches/ds-bot/files/test/connectors-app-secret-field.test.js b/patches/ds-bot/files/test/connectors-app-secret-field.test.js new file mode 100644 index 000000000..0f32aea40 --- /dev/null +++ b/patches/ds-bot/files/test/connectors-app-secret-field.test.js @@ -0,0 +1,96 @@ +// #2799: in Feishu's app mode the App Secret is a masked credential. The masked input is the Secrets +// page's bare `.bt-secret-input` — no border, no background — because it only reads as a field +// inside a `.bt-q-field` wrapper. The connector form drew it without one, so the App Secret appeared +// as a grey placeholder line under the App ID box and the form looked like "App ID only". +import assert from 'node:assert/strict' +import { test } from 'node:test' +import { fileURLToPath } from 'node:url' +import { runInNewContext } from 'node:vm' +import { build } from 'esbuild' +import { FEISHU_MODES } from '../src/host/connectors.js' + +// The client bundle is built once, as the Settings page loads it, and run with a small React stand-in +// whose hook state is fed per render (the same shape core/test/feishu.test.js uses). +const { outputFiles } = await build({ + stdin: { + contents: "export { ConnectorsPage } from './src/client/connectors.js'", + resolveDir: fileURLToPath(new URL('..', import.meta.url)), + }, + bundle: true, format: 'cjs', platform: 'browser', write: false, logLevel: 'silent', + external: ['react', 'react-dom', '@deepseek-ai/dsh-client-ui-primitives'], +}) +const h = (type, props, ...children) => ({ type, props: { ...props, children } }) +let states = [] +const react = { + createElement: h, Fragment: Symbol('F'), useEffect() {}, useLayoutEffect() {}, + useState: v => [states.length > 0 ? states.shift() : (typeof v === 'function' ? v() : v), () => {}], + useMemo: f => f(), useRef: current => ({ current }), useCallback: f => f, useSyncExternalStore: (_s, get) => get(), +} +const browser = { exports: {} } +runInNewContext(outputFiles[0].text, { + module: browser, exports: browser.exports, document: { hidden: false }, navigator: { language: 'en' }, localStorage: { getItem: () => null, setItem() {} }, + window: { matchMedia: () => ({ matches: false }) }, setTimeout, clearTimeout, + require: (name) => { + if (name === 'react') return react + if (name === 'react-dom') return { createPortal: node => node } + if (name === '@deepseek-ai/dsh-client-ui-primitives') return {} + throw new Error(`Unexpected platform module ${name}`) + }, +}) +const { ConnectorsPage } = browser.exports + +const expand = (node) => { + if (node === null || node === undefined || typeof node === 'boolean') return null + if (Array.isArray(node)) return node.map(expand) + if (typeof node !== 'object') return node + if (typeof node.type === 'function') return expand(node.type(node.props)) + return { ...node, props: { ...node.props, children: expand(node.props.children) } } +} +const all = (node, out = []) => { if (node && typeof node === 'object') { if (Array.isArray(node)) node.forEach(child => all(child, out)); else { out.push(node); all(node.props?.children, out) } } return out } +const text = node => (node === null || node === undefined ? '' : Array.isArray(node) ? node.map(text).join('') : typeof node === 'object' ? text(node.props.children) : String(node)) +const holds = (node, target) => node === target || (node && typeof node === 'object' && (Array.isArray(node) ? node.some(child => holds(child, target)) : holds(node.props?.children, target))) +const classes = node => String(node?.props?.className ?? '').split(/\s+/).filter(Boolean) + +const actions = { connectorConnect: async () => {}, connectorDisconnect: async () => {}, connectorRetry: async () => {}, connectorSend: async () => {} } +const FEISHU_OFF = { id: 'feishu', name: 'Feishu', state: 'off', kind: 'channel', account: null, mode: null, modes: FEISHU_MODES, allowed: [] } + +// ConnectorRow's hook order: form, bots, token, apps, picking, busy, error, message, copyState, mode, +// values. Everything after `mode` is left to its initializer. +const renderForm = (mode, values = {}) => { + states = [true, false, '', [], false, false, null, '', 'idle', mode, values] + const tree = expand(h(ConnectorsPage, { roster: { connectors: [{ ...FEISHU_OFF, mode }], bots: [], self: { memberId: 'me' } }, actions })) + states = [] + return tree +} + +const fieldWrapperOf = (tree, input) => all(tree).find(node => classes(node).includes('bt-q-field') && holds(node, input)) +const connectButton = tree => all(tree).find(node => node.type === 'button' && text(node) === 'Connect') + +test('#2799: app mode draws the masked App Secret inside the same bordered field wrapper as the App ID', () => { + const tree = renderForm('app') + const inputs = all(tree).filter(node => node.type === 'input') + assert.deepEqual(inputs.map(node => node.props['aria-label']), ['App ID', 'App Secret'], 'app mode asks for both credentials') + + const appId = inputs[0] + const secret = inputs[1] + const idWrapper = fieldWrapperOf(tree, appId) + const secretWrapper = fieldWrapperOf(tree, secret) + assert.ok(idWrapper, 'the App ID sits in a bordered .bt-q-field, as it always has') + assert.ok(secretWrapper, 'the App Secret field has no bordered wrapper: it renders as a bare .bt-secret-input, so the form reads as App ID only') + assert.deepEqual(classes(secretWrapper), ['bt-q-field', 'bt-secret-field'], 'the App Secret uses the same wrapper the Secrets page puts around MaskedInput') + assert.equal(secret.props.placeholder, 'Paste or type the key') + assert.equal(text(secretWrapper), '', 'the label stays the aria-label the field already carries, not a second visible row') +}) + +test('#2799: the webhook mode is unchanged — one masked field, still wrapped', () => { + const tree = renderForm('webhook') + const inputs = all(tree).filter(node => node.type === 'input') + assert.deepEqual(inputs.map(node => node.props['aria-label']), ['Webhook URL'], 'webhook mode still asks for one credential') + assert.ok(fieldWrapperOf(tree, inputs[0]), 'and that field keeps its bordered wrapper') +}) + +test('#2799: Connect stays disabled until both app fields have text', () => { + assert.equal(connectButton(renderForm('app', { app_id: 'cli_aa78608ba6f9dbb4' })).props.disabled, true, 'the App ID alone is not enough') + assert.equal(connectButton(renderForm('app', { app_secret: 'app-secret-value' })).props.disabled, true, 'nor is the App Secret alone') + assert.equal(connectButton(renderForm('app', { app_id: 'cli_aa78608ba6f9dbb4', app_secret: 'app-secret-value' })).props.disabled, false, 'both fields filled enable Connect') +})