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
1 change: 1 addition & 0 deletions changelog.d/2057-memory-workflows-dir-symlink.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
A memory or workflows folder that is a symlink is now read as empty instead of being followed ([#2057](https://github.com/ThinkFlowLab/FlyBot/issues/2057)): `<home>/memory/team`, `<home>/memory/bots/<id>` and a workspace's `workflows/` must be real directories for their files to be listed and read, so a link to a folder outside the home no longer feeds another folder's entries into a prompt, a listing or a page. The check runs before anything is listed or read; a folder that does not exist answers exactly what it answered before, a real folder is unchanged, and a linked one says so once per folder with one warning, because "nothing saved" and "nothing readable" look alike otherwise.
50 changes: 43 additions & 7 deletions core/src/host/memory.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import { codePointStart, cut } from '../shared/text-boundary.js'
import { output } from './tools.js'
import { retiredBot } from './store.js'
import { botNode, buildGraph, buildIndex, relatedOf, searchIndex, topicNode } from './memory-search.js'
import { dirKindSync } from './real-dir.js'

// Fixed sizes, in characters. Both MEMORY.md files ride in every request of every
// part, so together they stay near 8,000 characters, a few thousand tokens. A topic is
Expand Down Expand Up @@ -430,6 +431,31 @@ export function install(rt) {
const dirOf = scope => (scope.kind === 'team' ? join(root, 'team') : join(root, 'bots', safeId(scope.botId)))
const scopeKey = scope => (scope.kind === 'team' ? 'team' : `bot:${scope.botId}`)

// The memory folders are the home's own bookkeeping (#2057): `readdir` and `readFile` follow a
// directory link, so a linked folder would read another folder's files as this team's own. A
// reader answers exactly what it answers for a missing folder; a link is what differs, and it is
// said once per folder and process, because "nothing saved" and "nothing readable" look alike.
const warnedFolders = new Set()
const warnFolder = (kind, dir) => {
if (kind !== 'other' || warnedFolders.has(dir)) return
warnedFolders.add(dir)
rt.warn('ds-bot: memory folder is not a plain directory %s; keeping it empty', dir)
}
const scopeDir = (scope) => {
const base = dirKindSync(root)
if (base !== 'real') {
warnFolder(base, root)
return undefined
}
const dir = dirOf(scope)
const kind = dirKindSync(dir)
if (kind !== 'real') {
warnFolder(kind, dir)
return undefined
}
return dir
}

const readText = (path) => {
try {
return readFileSync(path, 'utf8')
Expand All @@ -438,22 +464,32 @@ export function install(rt) {
return ''
}
}
const topicNames = (scope) => {
const topicNamesIn = (dir) => {
try {
return readdirSync(dirOf(scope)).filter(name => name.endsWith('.md') && name !== MAIN).map(name => name.slice(0, -3)).sort()
return readdirSync(dir).filter(name => name.endsWith('.md') && name !== MAIN).map(name => name.slice(0, -3)).sort()
} catch {
return []
}
}
const topicNames = (scope) => {
const dir = scopeDir(scope)
return dir === undefined ? [] : topicNamesIn(dir)
}
// MEMORY.md first, then the topics by name.
const loadScope = (scope) => {
const dir = dirOf(scope)
return [undefined, ...topicNames(scope)].map((topic) => {
const open = scopeDir(scope) !== undefined
return [undefined, ...(open ? topicNamesIn(dir) : [])].map((topic) => {
const file = topic === undefined ? MAIN : `${topic}.md`
const content = readText(join(dir, file))
const content = open ? readText(join(dir, file)) : ''
return { topic, file, path: join(dir, file), content, ...parseMemory(content) }
})
}
// One scope's MEMORY.md, empty when its folder is not read.
const readMainOf = (scope) => {
const dir = scopeDir(scope)
return dir === undefined ? '' : readText(join(dir, MAIN))
}
const limitOf = file => (file.topic === undefined ? MEMORY_LIMITS.main : MEMORY_LIMITS.topic)
const fileName = file => (file.topic === undefined ? MAIN : `topic [[${file.topic}]]`)

Expand Down Expand Up @@ -683,8 +719,8 @@ export function install(rt) {
}
function memorySection(bot) {
const own = ownScope(bot.id)
const main = parseMemory(readText(join(dirOf(own), MAIN)))
const team = parseMemory(readText(join(dirOf(TEAM), MAIN)))
const main = parseMemory(readMainOf(own))
const team = parseMemory(readMainOf(TEAM))
return [
'# Your memory',
memoryRules(reviewOn),
Expand Down Expand Up @@ -1500,7 +1536,7 @@ export function install(rt) {
return {
// The head is the dossier's text, so its bound lands on a code point (#2005): a `slice`
// here handed the panel the first half of an emoji, and no later bound can put it back.
head: cut(readText(join(dirOf(scope), MAIN)), head).text,
head: cut(readMainOf(scope), head).text,
topics: topicNames(scope).slice(0, topics),
}
}
Expand Down
47 changes: 33 additions & 14 deletions core/src/host/workflow-store.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { copyOfPreset, isPresetId, isWorkflowId, workflowBytes, WORKFLOW_DIR } f
import { previewNode } from '../shared/workflow-preview.js'
import { repoDevWorkflow } from '../shared/workflow-repo-dev.js'
import { blankWorkflow } from '../shared/workflow-blank.js'
import { dirKindSync } from './real-dir.js'

// The shipped presets, as definitions. `PRESET_IDS` in `workflow-doc.js` names the same two; a test
// asserts they cannot drift apart.
Expand All @@ -36,21 +37,32 @@ const summary = (definition, source) => ({
columns: normaliseWorkflow(definition).nodes.filter((node) => node.column !== false).length,
})

// A directory may not exist yet (a workspace nobody has customized anything in), and a file in it
// may be someone's hand-edit that does not parse: one broken file must not take the list down.
const ownFiles = (dir) => {
try {
return readdirSync(dir).filter((name) => name.endsWith('.json') && name !== PROJECT_FILE).sort()
} catch {
return []
}
}

export function createWorkflowStore({ dir, presets = PRESETS, team = () => null, factsFor = () => ({ ok: false, why: 'this host has no fact reader' }), warn = () => {} } = {}) {
export function createWorkflowStore({ dir, presets = PRESETS, team = () => null, factsFor = () => ({ ok: false, why: 'this host has no fact reader' }), warn = () => {}, warnFolder = () => {} } = {}) {
if (typeof dir !== 'string' || dir === '') throw new Error('a workflow store needs a directory')
const fileFor = (id) => join(dir, `${id}.json`)

// `readdir` and `readFile` follow a directory link (#2057), so a `workflows` that is a link would
// list and read another folder's definitions as this workspace's own. Only a real folder is read;
// a missing or linked one answers what a missing folder answers today. The warning is what tells
// the two apart in the page, and the caller says it once per folder rather than once per read.
const folderReal = () => {
const kind = dirKindSync(dir)
if (kind !== 'real') warnFolder(dir, kind)
return kind === 'real'
}
// A directory may not exist yet (a workspace nobody has customized anything in), and a file in it
// may be someone's hand-edit that does not parse: one broken file must not take the list down.
const ownFiles = () => {
if (!folderReal()) return []
try {
return readdirSync(dir).filter((name) => name.endsWith('.json') && name !== PROJECT_FILE).sort()
} catch {
return []
}
}

const readOwn = (id) => {
if (!folderReal()) return null
try {
return normaliseWorkflow(JSON.parse(readFileSync(fileFor(id), 'utf8')))
} catch {
Expand All @@ -66,6 +78,7 @@ export function createWorkflowStore({ dir, presets = PRESETS, team = () => null,
}

const projectDefault = () => {
if (!folderReal()) return null
try {
const value = JSON.parse(readFileSync(join(dir, PROJECT_FILE), 'utf8'))
return typeof value?.default === 'string' && value.default !== '' ? value.default : null
Expand All @@ -75,7 +88,7 @@ export function createWorkflowStore({ dir, presets = PRESETS, team = () => null,
}

const list = () => {
const own = ownFiles(dir).flatMap((name) => {
const own = ownFiles().flatMap((name) => {
const definition = readOwn(name.slice(0, -'.json'.length))
if (definition === null || definition.id === '') {
warn('ds-bot: workflow store: %s in %s does not parse as a definition; skipped', name, dir)
Expand Down Expand Up @@ -175,7 +188,7 @@ export function createWorkflowStore({ dir, presets = PRESETS, team = () => null,
// The definitions in play, for the board's columns. Presets always; a workspace file is read on
// every call so a save is visible to the next board read without a restart.
definitions: () => {
const own = ownFiles(dir).map((name) => readOwn(name.slice(0, -'.json'.length))).filter((definition) => definition !== null && definition.id !== '')
const own = ownFiles().map((name) => readOwn(name.slice(0, -'.json'.length))).filter((definition) => definition !== null && definition.id !== '')
return [...presets.map(normaliseWorkflow), ...own]
},
bytesOf: (id) => {
Expand Down Expand Up @@ -206,10 +219,16 @@ export function install(rt) {
return null
}
}
const warnedFolders = new Set()
const warnFolder = (dir, kind) => {
if (kind !== 'other' || warnedFolders.has(dir)) return
warnedFolders.add(dir)
rt.warn?.('ds-bot: workflow folder is not a plain directory %s; keeping it empty', dir)
}
const store = () => {
const dir = dirFor()
if (dir === null) return null
if (rt.workflowStore?.dir !== dir) rt.workflowStore = createWorkflowStore({ dir, team, factsFor: rt.workflowFactsFor ?? (() => ({ ok: false, why: 'this host has no fact reader' })) })
if (rt.workflowStore?.dir !== dir) rt.workflowStore = createWorkflowStore({ dir, team, warn: rt.warn, warnFolder, factsFor: rt.workflowFactsFor ?? (() => ({ ok: false, why: 'this host has no fact reader' })) })
return rt.workflowStore
}
rt.workflowEndpoint = async (endpoint, body = {}) => {
Expand Down
65 changes: 64 additions & 1 deletion core/test/memory.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import assert from 'node:assert/strict'
import { existsSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
import { existsSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'
import { hostname, tmpdir } from 'node:os'
import { join } from 'node:path'
import { test } from 'node:test'
Expand Down Expand Up @@ -1391,3 +1391,66 @@ test('#2006: an ask prompt keeps whole characters at its 2,000-unit bound', asyn
assert.equal(sent.isWellFormed(), true, `the ask prompt keeps half an emoji: ${kept(sent)}`)
assert.ok(sent.endsWith(`${'q'.repeat(1999)}`), 'the prompt keeps the bound and drops the emoji whole')
})

// ---------------------------------------------------------------------------
// #2057: a linked folder is not this team's memory.

const captureWarnings = (t) => {
const lines = []
const original = console.error
console.error = (...args) => { lines.push(args.join(' ')) }
t.after(() => { console.error = original })
return () => lines.filter(line => line.includes('memory folder is not a plain directory'))
}

test('#2057: a team memory folder that is a symlink reads nothing, and says so once', async (t) => {
const home = mkdtempSync(join(tmpdir(), 'ds-bot-memory-'))
const outside = mkdtempSync(join(tmpdir(), 'ds-bot-memory-outside-'))
writeFileSync(join(outside, 'x.md'), '# x\n\n- an entry from outside the home\n')
mkdirSync(join(home, 'memory'), { recursive: true })
symlinkSync(outside, join(home, 'memory', 'team'), 'dir')
t.after(() => rmSync(outside, { recursive: true, force: true }))
const folderWarnings = captureWarnings(t)
const { section, call, stop } = start({ home })
t.after(() => stop())
// Three reads: the prompt section is what carries memory into every request.
for (let round = 0; round < 3; round++) {
const text = section('b2')
assert.doesNotMatch(text, /an entry from outside the home/)
assert.doesNotMatch(text, /\[\[x\]\]/)
const view = (await call('memory-view', { botId: 'b2' })).value
assert.deepEqual(view.entries.filter(entry => entry.scope === 'team'), [])
assert.equal(view.sizes.team, 0)
}
const warned = folderWarnings()
assert.equal(warned.length, 1, 'one warning per folder per process, not one per read')
assert.ok(warned[0].includes(join(home, 'memory', 'team')), `the warning names the folder: ${warned[0]}`)
assert.match(warned[0], /keeping it empty$/)
})

test('#2057: a memory folder that was never created reads as empty and says nothing', async (t) => {
const folderWarnings = captureWarnings(t)
const { section, call, stop, home } = start()
t.after(() => stop())
assert.equal(existsSync(join(home, 'memory')), false)
assert.doesNotMatch(section('b2'), /an entry from outside/)
const view = (await call('memory-view', { botId: 'b2' })).value
assert.deepEqual(view.entries, [])
assert.deepEqual(folderWarnings(), [])
})

test('#2057: a real team memory folder keeps listing and reading, with no warning', async (t) => {
const home = mkdtempSync(join(tmpdir(), 'ds-bot-memory-'))
mkdirSync(join(home, 'memory', 'team'), { recursive: true })
writeFileSync(join(home, 'memory', 'team', 'MEMORY.md'), '# Memory\n\n- Ships on Friday [added: 2026-11-01]\n')
writeFileSync(join(home, 'memory', 'team', 'travel.md'), '# travel\n\n- Flies from Hangzhou\n')
const folderWarnings = captureWarnings(t)
const { section, call, stop } = start({ home })
t.after(() => stop())
const view = (await call('memory-view', { botId: 'b2' })).value
const text = section('b2')
assert.match(text, /- Ships on Friday/)
assert.match(text, /Topics: \[\[travel\]\]/)
assert.deepEqual(view.entries.filter(entry => entry.scope === 'team').map(entry => entry.text), ['Ships on Friday', 'Flies from Hangzhou'])
assert.deepEqual(folderWarnings(), [])
})
85 changes: 84 additions & 1 deletion core/test/workflow-store.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// actor (acceptance 6 and 7).
import assert from 'node:assert/strict'
import { createHash } from 'node:crypto'
import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'
import { tmpdir } from 'node:os'
import { join } from 'node:path'
import { test } from 'node:test'
Expand Down Expand Up @@ -151,3 +151,86 @@ test('a workspace file that does not parse is skipped instead of breaking the li
assert.deepEqual(listed.own.map((entry) => entry.id), ['blank-mine'])
})
})

// ---------------------------------------------------------------------------
// #2057: a linked folder is not this workspace's workflows.

const elsewhere = (t, files) => {
const root = mkdtempSync(join(tmpdir(), 'flybot-workflows-outside-'))
for (const [name, text] of Object.entries(files)) writeFileSync(join(root, name), text)
t.after(() => rmSync(root, { recursive: true, force: true }))
return root
}

test('#2057: a workflow folder that is a symlink lists nothing and never reads the outside file', (t) => {
const root = mkdtempSync(join(tmpdir(), 'flybot-workflows-'))
t.after(() => rmSync(root, { recursive: true, force: true }))
const dir = join(root, 'workflows')
const outside = elsewhere(t, { 'other.json': JSON.stringify({ ...blankWorkflow, id: 'other', name: 'Other' }) })
symlinkSync(outside, dir, 'dir')
const store = createWorkflowStore({ dir })
const listed = store.list()
assert.deepEqual(listed.own, [])
assert.equal(listed.default, null)
const opened = store.open('other')
assert.equal(opened.ok, false)
assert.equal(opened.code, 'workflow/not-found')
assert.equal(JSON.stringify(opened).includes('Other'), false, 'the outside definition never comes back')
assert.deepEqual(store.definitions().map((definition) => definition.id).sort(), PRESETS.map((preset) => preset.id).sort())
})

test('#2057: a workflow folder that is a plain file lists nothing and does not throw', (t) => {
const root = mkdtempSync(join(tmpdir(), 'flybot-workflows-'))
t.after(() => rmSync(root, { recursive: true, force: true }))
const dir = join(root, 'workflows')
writeFileSync(dir, 'not a folder')
const store = createWorkflowStore({ dir })
assert.deepEqual(store.list().own, [])
assert.deepEqual(store.definitions().map((definition) => definition.id).sort(), PRESETS.map((preset) => preset.id).sort())
})

test('#2057: a linked workflow folder warns once, a missing or real one never warns', async (t) => {
const warned = []
const host = (root) => {
const rt = {
state: { bots: {} },
ctx: { workspaceRegistry: { get: () => ({ path: root }) } },
warn: (template, ...args) => warned.push(String(template).replace(/%s/g, () => String(args.shift() ?? ''))),
}
install(rt)
return rt
}
const folderWarnings = () => warned.filter((line) => line.includes('workflow folder is not a plain directory'))

// A link to a folder outside the workspace: every read says "nothing", and the folder is named once.
const linkedRoot = mkdtempSync(join(tmpdir(), 'flybot-workflows-'))
t.after(() => rmSync(linkedRoot, { recursive: true, force: true }))
const outside = elsewhere(t, { 'other.json': JSON.stringify({ ...blankWorkflow, id: 'other', name: 'Other' }) })
symlinkSync(outside, join(linkedRoot, 'workflows'), 'dir')
const linked = host(linkedRoot)
for (let round = 0; round < 3; round++) {
const listed = await linked.workflowEndpoint('workflows', { op: 'list' })
assert.deepEqual(listed.own, [])
linked.workflowDefinitions()
}
const said = folderWarnings()
assert.equal(said.length, 1, 'one warning per folder per process, not one per read')
assert.ok(said[0].includes(join(linkedRoot, 'workflows')), `the warning names the folder: ${said[0]}`)
assert.match(said[0], /keeping it empty$/)

// A workspace nobody has saved a workflow in: empty, silent.
const freshRoot = mkdtempSync(join(tmpdir(), 'flybot-workflows-'))
t.after(() => rmSync(freshRoot, { recursive: true, force: true }))
const fresh = host(freshRoot)
assert.deepEqual((await fresh.workflowEndpoint('workflows', { op: 'list' })).own, [])
assert.deepEqual(folderWarnings(), said)

// A real folder: listed, silent.
const realRoot = mkdtempSync(join(tmpdir(), 'flybot-workflows-'))
t.after(() => rmSync(realRoot, { recursive: true, force: true }))
mkdirSync(join(realRoot, 'workflows'), { recursive: true })
writeFileSync(join(realRoot, 'workflows', 'mine.json'), JSON.stringify({ ...blankWorkflow, id: 'mine', name: 'Mine' }))
const real = host(realRoot)
assert.deepEqual((await real.workflowEndpoint('workflows', { op: 'list' })).own.map((entry) => entry.id), ['mine'])
assert.deepEqual(folderWarnings(), said)
})
Loading
Loading