Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 81 additions & 0 deletions .github/scripts/check-empty-catch.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
// A catch clause whose block holds neither a statement nor a comment swallows the
// error with nothing on record saying why that is safe (#205). The comment is the
// reason; a block with statements handles the error and needs none. Only the
// statement form is checked: the `void p.catch(() => {})` promise form is a separate
// fire-and-forget idiom, and test helpers may retry through a bare catch.
// Usage: node .github/scripts/check-empty-catch.mjs (exit 1 lists every violation)
import { readdirSync, readFileSync } from 'node:fs'
import { join } from 'node:path'

const CATCH = /(?<![\w$.])catch\s*(?:\([^)]*\))?\s*\{/g

// The block after `{`, ending at the brace that closes it. Braces inside strings can
// only end the walk early, and a truncated block still shows its leading statements,
// so the walk never invents a violation that is not there.
const blockOf = (source, open) => {
let depth = 0
for (let at = open; at < source.length; at += 1) {
if (source[at] === '{') depth += 1
if (source[at] === '}') {
depth -= 1
if (depth === 0) return source.slice(open + 1, at)
}
}
return source.slice(open + 1)
}

// What remains of the block once comments are gone; `comment` says whether any was.
const stripComments = (block) => {
let comment = false
const code = block.replace(/\/\/[^\n]*|\/\*[\s\S]*?\*\//g, (found) => {
comment = true
return ''
})
return { code, comment }
}

export function scanSource(name, source) {
const found = []
for (const match of source.matchAll(CATCH)) {
const open = match.index + match[0].length - 1
const { code, comment } = stripComments(blockOf(source, open))
if (code.trim() === '' && !comment) {
const line = source.slice(0, match.index).split('\n').length
found.push(`${name}:${line}`)
}
}
return found
}

const under = (dir, names) => {
const out = []
for (const entry of readdirSync(dir, { withFileTypes: true })) {
if (entry.name === 'node_modules' || entry.name === 'dist' || entry.name === 'dev' || entry.name === 'test') continue
const path = join(dir, entry.name)
if (entry.isDirectory()) out.push(...under(path, names))
else if (entry.isFile() && entry.name.endsWith('.js')) out.push(path)
}
return out
}

export function collectTargets(root) {
const files = [join(root, 'core/index.js'), join(root, 'core/web-search.js'), ...under(join(root, 'core/src'))]
for (const plugin of readdirSync(join(root, 'plugins'), { withFileTypes: true })) {
if (!plugin.isDirectory()) continue
const dir = join(root, 'plugins', plugin.name)
for (const name of ['index.js', 'client.js']) files.push(join(dir, name))
files.push(...under(join(dir, 'src')))
}
return files
}

export function scanTree(root) {
return collectTargets(root).flatMap((path) => scanSource(path, readFileSync(path, 'utf8')))
}

if (process.argv[1] === import.meta.filename) {
const root = join(import.meta.dirname, '..', '..')
const violations = scanTree(root)
for (const where of violations) console.log(`${where}: empty catch: the error is swallowed with no reason on record`)
if (violations.length > 0) process.exit(1)
}
27 changes: 27 additions & 0 deletions .github/scripts/check-empty-catch.test.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { test } from 'node:test'
import assert from 'node:assert/strict'
import { join } from 'node:path'
import { scanSource, scanTree } from './check-empty-catch.mjs'

test('scanSource: a catch block with no statement and no comment is a violation', () => {
assert.deepEqual(scanSource('a.js', 'try { x() } catch {}\n'), ['a.js:1'])
assert.deepEqual(scanSource('a.js', 'try { x() } catch (e) {}\n'), ['a.js:1'])
assert.deepEqual(scanSource('a.js', 'try { x() } catch ({ message }) {}\n'), ['a.js:1'])
assert.deepEqual(scanSource('a.js', 'try { x() } catch ({}) {}\n'), ['a.js:1'])
assert.deepEqual(scanSource('a.js', 'try {\n x()\n} catch {\n\n}\n'), ['a.js:3'])
})

test('scanSource: a comment in the block is the reason; statements handle the error', () => {
assert.deepEqual(scanSource('a.js', 'try { x() } catch { /* racing unlink */ }\n'), [])
assert.deepEqual(scanSource('a.js', 'try { x() } catch (e) { return }\n'), [])
assert.deepEqual(scanSource('a.js', 'try { x() } catch {\n // retries on the next poll\n}\n'), [])
})

test('scanSource: the promise form, plain blocks and other identifiers are out of scope', () => {
assert.deepEqual(scanSource('a.js', 'void save().catch(() => {})\n'), [])
assert.deepEqual(scanSource('a.js', 'for (;;) {}\nconst o = { catch: 1 }\n'), [])
})

test('the shipped sources carry no bare catch', () => {
assert.deepEqual(scanTree(join(import.meta.dirname, '..', '..')), [])
})
2 changes: 1 addition & 1 deletion core/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export function provideFlyBot(ctx, flybot) {
try {
ctx.set('flybot', flybot)
return
} catch {}
} catch { /* the key is refused: fall through to the plain property */ }
}
ctx.flybot = flybot
}
Expand Down
2 changes: 1 addition & 1 deletion core/src/client/flybot-registry.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ export function createClientApi({ call: callFn, registry: slots = registry, ever
const { seq: _seq, ...event } = change
for (const fn of [...listeners]) fn(event)
}
} catch {}
} catch { /* a failed poll leaves the fields as they are; the next one catches up */ }
}
return {
bots: () => callFn('bots'),
Expand Down
2 changes: 1 addition & 1 deletion core/src/client/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export function apply(ctx) {
if (typeof ctx.provide === 'function') ctx.provide('flybot', flybot)
else if (typeof ctx.reflect?.provide === 'function') ctx.reflect.provide('flybot', flybot)
else if (typeof ctx.set === 'function') ctx.set('flybot', flybot)
} catch {}
} catch { /* every provide form refused: __flybot__.api above still serves plugins */ }
ctx.effect(() => installLocale(ctx.get('locale')), 'ds-bot: interface copy')
ctx.effect(() => installBrandChrome(), 'ds-bot: brand chrome')
const roster = createSource(EMPTY_ROSTER)
Expand Down
4 changes: 2 additions & 2 deletions core/src/host/diagnostics.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,9 @@ const dshVersion = () => {
try {
const manifest = JSON.parse(readFileSync(join(dir, 'package.json'), 'utf8'))
if (manifest.name === '@deepseek-ai/dsh-llm') return manifest.version
} catch {}
} catch { /* no readable manifest at this depth: keep walking up */ }
}
} catch {}
} catch { /* the package does not resolve here: no version to report */ }
return undefined
}

Expand Down
6 changes: 3 additions & 3 deletions core/src/host/home-lock.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export function homeLock(path, { staleMs = 60_000, beatMs = 15_000, host = hostn
return
}
const time = new Date(now())
try { utimesSync(path, time, time) } catch {}
try { utimesSync(path, time, time) } catch { /* best effort: a stale mtime only lets a live check retake it */ }
}
const stop = () => {
if (timer !== undefined) clearInterval(timer)
Expand Down Expand Up @@ -80,7 +80,7 @@ export function homeLock(path, { staleMs = 60_000, beatMs = 15_000, host = hostn
? current?.token === holder.token
: JSON.stringify(current) === JSON.stringify(holder)
if (unchanged) {
try { unlinkSync(path) } catch {}
try { unlinkSync(path) } catch { /* another claimer removed it first */ }
}
}
return read() ?? {}
Expand All @@ -89,7 +89,7 @@ export function homeLock(path, { staleMs = 60_000, beatMs = 15_000, host = hostn
function release() {
stop()
if (owns()) {
try { unlinkSync(path) } catch {}
try { unlinkSync(path) } catch { /* already gone, or taken over */ }
}
}

Expand Down
2 changes: 1 addition & 1 deletion core/src/host/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -816,7 +816,7 @@ export function install(rt) {
// A message the user addressed to a friend's Bot is that Bot's to answer; its
// reply comes back into this chat. The local Bot does not answer it itself.
if (!results.some(result => result.action !== 'deny') || rt.roomOf(id)) return
try { (await rt.resolveAgent(id)).cancel({ kind: 'user' }) } catch {}
try { (await rt.resolveAgent(id)).cancel({ kind: 'user' }) } catch { /* ended or already gone: nothing to cancel */ }
})
.catch(error => warn('flybot: user mention failed: %s', error))
})
Expand Down
5 changes: 5 additions & 0 deletions docs/code-standards.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ When you report status on a change:
- If review or a test finds one escape path, fix the shared helper and cover sibling markers (`untrusted-message`, `untrusted-file`, trusted headers).
- Same-class reds tighten LESSONS / this file; do not paper over with a special case.

## Silent catches

- A `catch` that swallows the error says why in a comment inside the block (or handles it); `check-empty-catch` in `.github/scripts/` fails the build on a bare one (#205).
- The `void p.catch(() => {})` promise form is a separate fire-and-forget idiom and test-helper retries stay out of the gate's scope.

## Security: fences and remote strings

- Untrusted bodies stay inside fences; closing markers must not reconstitute after a single strip.
Expand Down
12 changes: 12 additions & 0 deletions patches/ds-bot/0001-core-fixes.patch
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,18 @@ diff --git a/src/host/diagnostics.js b/src/host/diagnostics.js
row('DSH', facts.dshVersion),
row('Surface', client.surface),
row('OS', facts.os),
@@ -105,9 +105,9 @@ const dshVersion = () => {
try {
const manifest = JSON.parse(readFileSync(join(dir, 'package.json'), 'utf8'))
if (manifest.name === '@deepseek-ai/dsh-llm') return manifest.version
- } catch {}
+ } catch { /* no readable manifest at this depth: keep walking up */ }
}
- } catch {}
+ } catch { /* the package does not resolve here: no version to report */ }
return undefined
}

@@ -167,7 +167,9 @@ export function install(rt) {
const error = args.find(arg => arg instanceof Error)
const line = clean(format(template, ...args.map(arg => (arg instanceof Error ? errorText(arg) : arg))))
Expand Down
32 changes: 31 additions & 1 deletion patches/ds-bot/0002-flybot-api-host.patch
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,42 @@ diff --git a/index.js b/index.js
+ try {
+ ctx.set('flybot', flybot)
+ return
+ } catch {}
+ } catch { /* the key is refused: fall through to the plain property */ }
+ }
+ ctx.flybot = flybot
}

export function createRuntime(ctx, config = {}) {
diff --git a/src/host/home-lock.js b/src/host/home-lock.js
--- a/src/host/home-lock.js
+++ b/src/host/home-lock.js
@@ -50,7 +50,7 @@ export function homeLock(path, { staleMs = 60_000, beatMs = 15_000, host = hostn
return
}
const time = new Date(now())
- try { utimesSync(path, time, time) } catch {}
+ try { utimesSync(path, time, time) } catch { /* best effort: a stale mtime only lets a live check retake it */ }
}
const stop = () => {
if (timer !== undefined) clearInterval(timer)
@@ -80,7 +80,7 @@ export function homeLock(path, { staleMs = 60_000, beatMs = 15_000, host = hostn
? current?.token === holder.token
: JSON.stringify(current) === JSON.stringify(holder)
if (unchanged) {
- try { unlinkSync(path) } catch {}
+ try { unlinkSync(path) } catch { /* another claimer removed it first */ }
}
}
return read() ?? {}
@@ -89,7 +89,7 @@ export function homeLock(path, { staleMs = 60_000, beatMs = 15_000, host = hostn
function release() {
stop()
if (owns()) {
- try { unlinkSync(path) } catch {}
+ try { unlinkSync(path) } catch { /* already gone, or taken over */ }
}
}

diff --git a/src/host/roster.js b/src/host/roster.js
--- a/src/host/roster.js
+++ b/src/host/roster.js
Expand Down
2 changes: 1 addition & 1 deletion patches/ds-bot/0003-flybot-api-client.patch
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,7 @@ diff --git a/src/client/index.js b/src/client/index.js
+ if (typeof ctx.provide === 'function') ctx.provide('flybot', flybot)
+ else if (typeof ctx.reflect?.provide === 'function') ctx.reflect.provide('flybot', flybot)
+ else if (typeof ctx.set === 'function') ctx.set('flybot', flybot)
+ } catch {}
+ } catch { /* every provide form refused: __flybot__.api above still serves plugins */ }
ctx.effect(() => installLocale(ctx.get('locale')), 'ds-bot: interface copy')
+ ctx.effect(() => installBrandChrome(), 'ds-bot: brand chrome')
const roster = createSource(EMPTY_ROSTER)
Expand Down
2 changes: 1 addition & 1 deletion patches/ds-bot/files/src/client/flybot-registry.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ export function createClientApi({ call: callFn, registry: slots = registry, ever
const { seq: _seq, ...event } = change
for (const fn of [...listeners]) fn(event)
}
} catch {}
} catch { /* a failed poll leaves the fields as they are; the next one catches up */ }
}
return {
bots: () => callFn('bots'),
Expand Down
2 changes: 1 addition & 1 deletion patches/ds-bot/files/src/host/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -816,7 +816,7 @@ export function install(rt) {
// A message the user addressed to a friend's Bot is that Bot's to answer; its
// reply comes back into this chat. The local Bot does not answer it itself.
if (!results.some(result => result.action !== 'deny') || rt.roomOf(id)) return
try { (await rt.resolveAgent(id)).cancel({ kind: 'user' }) } catch {}
try { (await rt.resolveAgent(id)).cancel({ kind: 'user' }) } catch { /* ended or already gone: nothing to cancel */ }
})
.catch(error => warn('flybot: user mention failed: %s', error))
})
Expand Down
1 change: 1 addition & 0 deletions patches/ds-bot/topics.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
"src/host/store.js",
"src/host/roster.js",
"src/host/tools.js",
"src/host/home-lock.js",
"test/fake-ctx.js"
],
"0003-flybot-api-client": [
Expand Down
2 changes: 1 addition & 1 deletion plugins/social/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -897,7 +897,7 @@
await clip?.writeText?.(handle)
setCopied(true)
setTimeout(() => setCopied(false), 1500)
} catch {}
} catch { /* clipboard denied: skip the Copied feedback */ }
}
return h('div', { className: 'fs-me', 'data-flybot': 'my-handle-menu' },
h('span', { className: 'fs-me-handle', title: 'Your Bot Social handle' }, handle),
Expand Down Expand Up @@ -1354,7 +1354,7 @@
status = { at: Date.now(), value }
if (changed) for (const fn of [...statusListeners]) { try { fn() } catch (error) { console.error('flybot-social: status listener failed', error) } }
return value
}).finally(() => { statusTask = undefined })

Check warning on line 1357 in plugins/social/client.js

View workflow job for this annotation

GitHub Actions / plugins (social)

unicorn(no-useless-spread)

Using a spread operator here creates a new array unnecessarily.
return statusTask
}
// Fires when what canInvite answers may have changed (first status, connect/disconnect, Bots added).
Expand Down
4 changes: 2 additions & 2 deletions plugins/social/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ export function apply(ctx, config = {}) {
.catch((error) => {
const reason = startFailureReason(error)
console.warn('flybot-social: start failed:', reason)
try { ctx.logger?.warn?.('flybot-social: start failed: %s', reason) } catch {}
try { ctx.logger?.warn?.('flybot-social: start failed: %s', reason) } catch { /* the logger failed while reporting the failure */ }
// Host diagnostics (Settings → Feedback). Optional on older cores without flybot.warn.
try { ctx.flybot?.warn?.('flybot-social: start failed: %s', error) } catch {}
try { ctx.flybot?.warn?.('flybot-social: start failed: %s', error) } catch { /* the optional channel itself failed */ }
if (!stopped) unavailableOff = registerUnavailable(ctx.flybot, reason)
})
ctx.effect(() => () => {
Expand Down
2 changes: 1 addition & 1 deletion plugins/social/src/mcp-client.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
const lib = parsed.protocol === 'http:' ? http : https
const agent = init.agent ?? agentFor(url)
const method = String(init.method ?? 'GET').toUpperCase()
const headers = { ...(init.headers ?? {}) }

Check warning on line 44 in plugins/social/src/mcp-client.js

View workflow job for this annotation

GitHub Actions / plugins (social)

unicorn(no-useless-fallback-in-spread)

Empty fallbacks in spreads are unnecessary
const signal = init.signal
return new Promise((resolve, reject) => {
const req = lib.request(parsed, { method, headers, agent }, (res) => {
Expand Down Expand Up @@ -76,7 +76,7 @@
const list = [...items]
if (list.length === 0) return []
const cap = Math.max(1, Math.min(limit || GRANT_CONCURRENCY, list.length))
const out = new Array(list.length)

Check warning on line 79 in plugins/social/src/mcp-client.js

View workflow job for this annotation

GitHub Actions / plugins (social)

unicorn(no-new-array)

Do not use `new Array(singleArgument)`.
let next = 0
async function worker() {
for (;;) {
Expand Down Expand Up @@ -145,7 +145,7 @@
if (/text\/event-stream/i.test(type) || /^(event|data|id):/m.test(text.trimStart().slice(0, 6))) {
const data = text.split(/\r?\n/).filter(line => line.startsWith('data:')).map(line => line.slice(5).trim()).filter(Boolean)
for (const line of data.reverse()) {
try { const value = JSON.parse(line); if (value && (value.result !== undefined || value.error !== undefined)) return value } catch {}
try { const value = JSON.parse(line); if (value && (value.result !== undefined || value.error !== undefined)) return value } catch { /* not the JSON-RPC answer: try the next data line */ }
}
throw new BotSocialError('Bot Social: the event stream had no answer', 'sse')
}
Expand Down
6 changes: 3 additions & 3 deletions plugins/social/src/social.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,9 @@
const TEXTUAL = new Set(['text/plain', 'text/markdown', 'text/csv', 'text/tab-separated-values', 'application/json'])
const sizeLabel = n => (n < 1024 ? `${n} B` : n < 1048576 ? `${(n / 1024).toFixed(1)} KB` : `${(n / 1048576).toFixed(1)} MB`)
// Remote strings that reach model-visible text: one line, no brackets or markup, capped.
export const cleanText = (value, fallback = '') => String(value ?? '').replace(/[\u0000-\u001f\u007f-\u009f\u2028\u2029]/g, ' ').replace(/[[\]"<>`]/g, '').replace(/\s+/g, ' ').trim().slice(0, 60) || fallback

Check warning on line 84 in plugins/social/src/social.js

View workflow job for this annotation

GitHub Actions / plugins (social)

eslint(no-control-regex)

Unexpected control characters
// Owner-only draft body: keep punctuation the friend will see; strip controls and cap.
export const draftText = (value) => String(value ?? '').replace(/[\u0000-\u001f\u007f-\u009f\u2028\u2029]/g, ' ').replace(/\s+/g, ' ').trim().slice(0, 800)

Check warning on line 86 in plugins/social/src/social.js

View workflow job for this annotation

GitHub Actions / plugins (social)

eslint(no-control-regex)

Unexpected control characters
// Owner cards never show the FLYBOT-FACE-HUMAN wire envelope (#159).
export function approvalDraftText(value) {
const raw = draftText(value)
Expand Down Expand Up @@ -136,7 +136,7 @@
}
const SEEN_KEPT = 1000
const CREDENTIAL_NAME = /(^id_(rsa|dsa|ecdsa|ed25519)|\.(pem|key|p12|pfx|kdbx|keychain)$|credential|secret|token|password|passwd|^flybot-social.*\.json$|^\.?env$|^\.netrc$|^known_hosts$|^authorized_keys$)/i
const safeName = name => String(name ?? 'file').replace(/[\u0000-\u001f\u007f/\\:]/g, '_').replace(/^\.+/, '_').slice(0, 120) || 'file'

Check warning on line 139 in plugins/social/src/social.js

View workflow job for this annotation

GitHub Actions / plugins (social)

eslint(no-control-regex)

Unexpected control characters
// The text Copy puts on the clipboard for a per-Bot invite: what to do with the link, for a
// person in FlyBot or for their Grok bot.
export function inviteText({ handle, name, home, url, expiresAt }) {
Expand Down Expand Up @@ -294,7 +294,7 @@
export const writeCredentials = (path, value) => {
mkdirSync(dirname(path), { recursive: true })
writeFileSync(path, JSON.stringify(value, null, 2), { mode: 0o600 })
try { chmodSync(path, 0o600) } catch {}
try { chmodSync(path, 0o600) } catch { /* file systems that ignore the write mode */ }
}

export function createSocial(service, config = {}, { client: given, api: givenApi, home: givenHome, now = Date.now, setInterval: every = globalThis.setInterval, clearInterval: clear = globalThis.clearInterval, faceApi: givenFace } = {}) {
Expand Down Expand Up @@ -389,7 +389,7 @@
if (CREDENTIAL_NAME.test(basename(file))) return 'it looks like a credential'
for (const dir of privateDirs()) {
let real = dir
try { real = realpathSync(dir) } catch {}
try { real = realpathSync(dir) } catch { /* not a symlink or gone: compare the path as it is */ }
if (file === real || file.startsWith(real + sep)) return 'it is a private FlyBot file'
}
if (!(extname(file).toLowerCase() in MIME)) return `only ${Object.keys(MIME).join(' ')} files`
Expand Down Expand Up @@ -575,7 +575,7 @@
if (result?.unavailable) return
} catch (error) {
if (/invalid_avatar|invalid_color|invalid_request/i.test(`${error?.code ?? ''} ${error?.message ?? ''}`)) {
settings.lookSynced = { ...(settings.lookSynced ?? {}), [bot.id]: fingerprint }

Check warning on line 578 in plugins/social/src/social.js

View workflow job for this annotation

GitHub Actions / plugins (social)

unicorn(no-useless-fallback-in-spread)

Empty fallbacks in spreads are unnecessary
saveSettings()
return
}
Expand Down Expand Up @@ -1097,7 +1097,7 @@
try {
const message = await client.callTool('send_message', { conversation_id: bot.conversationId, text: msg.text })
return { exchangeId: message?.message_id ?? msg.exchangeId }
} catch {}
} catch { /* the old conversation is gone: start a fresh one below */ }
}
const started = await client.callTool('start_conversation', { friend: bot.handle, text: msg.text, ...(config.botId ? { bot_id: config.botId } : {}) })
// Kept, so a file sent right after goes into this conversation.
Expand Down
Loading