From 886417d70c2a3ea9fe3e515cbd3ce92a23b18da9 Mon Sep 17 00:00:00 2001 From: Day Dreamer <87640074+HugoXK@users.noreply.github.com> Date: Thu, 5 Nov 2026 03:39:44 +0800 Subject: [PATCH] host: name a Project State file the reader cannot read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `listProjectState` caught every failure of `describeFile` into one silent `catch`, so a folder whose PROJECT.md is a directory, or a file this process may not read, answered `no Project State files in this folder` — the one answer that says there is nothing to read. The listing now carries the name and the reason beside the rows that could be read, `unreadable: PROJECT.md (EACCES)`; an absent name, and a symlink, still add nothing. Fixes #2461 --- .../2461-project-state-unreadable.fixed.md | 1 + core/src/host/project-state.js | 64 +++++++--- core/test/project-state-unreadable.test.js | 114 ++++++++++++++++++ docs/subsystems/core-host.md | 2 +- .../ds-bot/files/src/host/project-state.js | 64 +++++++--- .../test/project-state-unreadable.test.js | 114 ++++++++++++++++++ 6 files changed, 328 insertions(+), 31 deletions(-) create mode 100644 changelog.d/2461-project-state-unreadable.fixed.md create mode 100644 core/test/project-state-unreadable.test.js create mode 100644 patches/ds-bot/files/test/project-state-unreadable.test.js diff --git a/changelog.d/2461-project-state-unreadable.fixed.md b/changelog.d/2461-project-state-unreadable.fixed.md new file mode 100644 index 000000000..67cdf739a --- /dev/null +++ b/changelog.d/2461-project-state-unreadable.fixed.md @@ -0,0 +1 @@ +`project_state` no longer reports a Project State file it cannot read as if the folder held none: a `PROJECT.md` that is a folder, or a file the process may not read, now answers `unreadable: PROJECT.md ()` beside the rows that could be read instead of the listing answering `no Project State files in this folder`, which said there was nothing to read while something was ([#2461](https://github.com/ThinkFlowLab/FlyBot/issues/2461)). diff --git a/core/src/host/project-state.js b/core/src/host/project-state.js index 19b98c1f9..e3d45d117 100644 --- a/core/src/host/project-state.js +++ b/core/src/host/project-state.js @@ -67,13 +67,20 @@ async function decisionFiles(dir) { // is what `project_state` and the board's opened row already answer for a missing one. async function fileAt(dir, path) { if (isDecisionPath(path) && await dirKind(dirname(join(dir, path))) !== 'real') { - throw new Error(`not a plain file: ${path}`) + throw notPlainFile(path, true) } const info = await lstat(join(dir, path)) - if (!info.isFile()) throw new Error(`not a plain file: ${path}`) + if (!info.isFile()) throw notPlainFile(path, info.isSymbolicLink()) return info } +// A name that is there but is not a plain file. `absent` marks the shapes this reader already +// answers as a missing file — a symlink (#2055) and a decision whose `decisions/` is not a real +// folder here — so the listing drops them exactly like an absent name. Anything else (a folder, a +// fifo, a device) is a name the reader cannot read, which the listing answers instead of dropping +// (#2461): a folder whose `PROJECT.md` is a directory is not a folder with no Project State file. +const notPlainFile = (path, absent) => Object.assign(new Error(`not a plain file: ${path}`), { absent }) + // One Project State file with its byte size, content hash and last-modified time. Exported // because the board reads the file a decided meeting archived, and it must refuse exactly what // this reader refuses — one rule, one place. @@ -90,20 +97,40 @@ export async function describeFile(dir, path) { // The repository gate belongs to the tool, not to every reader (#1809): a decision file is the // board's own artifact and shows up in a folder that is not a repository, while `project_state` // keeps refusing such a folder by default. Only the board passes `{ repository: false }`. +// The reason a listed name has no row, short enough to sit inside one: the system's own errno when +// it gave one, else the rule the name broke (a folder where a plain file belongs). The name itself +// stays in the row, so the reason never repeats it and never carries a parenthesis of its own. +const unreadableReason = (error) => { + if (typeof error?.code === 'string' && error.code !== '') return error.code + const message = String(error?.message ?? error ?? '').split('\n')[0] + return message.startsWith('not a plain file') ? 'not a plain file' : (message.replace(/[()]/g, '') || 'unknown') +} + +// One listed name: a row with its byte size, hash and mtime, or the reason it has none. A name that +// is absent — or is a link, which this reader reads as absent (#2055) — adds nothing; a name that is +// there and could not be read is `unreadable: ()`, never a silent omission (#2461). +async function listedRow(dir, path) { + try { + return { file: await describeFile(dir, path) } + } catch (error) { + if (error?.absent === true || error?.code === 'ENOENT' || error?.code === 'ENOTDIR') return null + return { unreadable: { path, reason: unreadableReason(error) } } + } +} + export async function listProjectState(dir, { repository = true } = {}) { if (repository && !await repositoryAt(dir)) return { ok: false, reason: `not a repository: ${dir}` } const found = [] - for (const path of PROJECT_STATE_PATHS) { - try { - found.push(await describeFile(dir, path)) - } catch { /* an absent Project State file is simply not listed */ } - } - for (const path of await decisionFiles(dir)) { - try { - found.push(await describeFile(dir, path)) - } catch { /* a file that vanished between readdir and read is not listed */ } + const unreadable = [] + const add = async (path) => { + const row = await listedRow(dir, path) + if (row === null) return + if (row.file !== undefined) found.push(row.file) + else unreadable.push(row.unreadable) } - return { ok: true, dir, files: found } + for (const path of PROJECT_STATE_PATHS) await add(path) + for (const path of await decisionFiles(dir)) await add(path) + return { ok: true, dir, files: found, unreadable } } // The bytes of one named Project State file, bounded at `MAX_BYTES`. The path is checked @@ -145,14 +172,21 @@ export function workDirFor(rt, exec) { return rt.teamFolder?.() } -const listed = ({ files }) => files.length === 0 +// The answer the tool prints: the rows a caller compares hashes with, and — after them — the names +// that are there but could not be read. `none` is the answer for a folder with nothing to read, +// which is exactly what an unreadable name is not (#2461). +const listed = ({ files, unreadable }) => files.length === 0 && unreadable.length === 0 ? 'project state: no Project State files in this folder' - : ['project state:', ...files.map(file => `${file.path}\t${file.bytes} bytes\tsha256:${file.sha256}\t${file.mtime}`)].join('\n') + : [ + 'project state:', + ...files.map(file => `${file.path}\t${file.bytes} bytes\tsha256:${file.sha256}\t${file.mtime}`), + ...unreadable.map(({ path, reason }) => `unreadable: ${path} (${reason})`), + ].join('\n') export function install(rt) { rt.ctx.tools.register(defineTool({ name: 'project_state', - description: 'Read the Project State files of your working folder: PROJECT.md, ROADMAP.md, docs/governance.md, decisions/*.md, LESSONS.md. Without `file` it lists each with its byte size, sha256 content hash and last-modified time, so you can compare the hash with the copy you already have. With `file` it returns that one file, bounded. Answers `absent`, `too large` or `not a repository` with the reason instead of an empty document.', + description: 'Read the Project State files of your working folder: PROJECT.md, ROADMAP.md, docs/governance.md, decisions/*.md, LESSONS.md. Without `file` it lists each with its byte size, sha256 content hash and last-modified time, so you can compare the hash with the copy you already have. With `file` it returns that one file, bounded. Answers `absent`, `too large` or `not a repository` with the reason instead of an empty document, and a listed name that is there but cannot be read as `unreadable: ()` rather than leaving it out.', parameters: { file: { type: 'string', description: 'Optional Project State path to read, e.g. decisions/x.md; omit to list them all' }, }, diff --git a/core/test/project-state-unreadable.test.js b/core/test/project-state-unreadable.test.js new file mode 100644 index 000000000..82c9a4382 --- /dev/null +++ b/core/test/project-state-unreadable.test.js @@ -0,0 +1,114 @@ +// A Project State file that is there but cannot be read must not be dropped on the floor. The +// reader already answers `not a repository`, `absent` and `too large` with the reason instead of +// an empty document; a listed name that exists and fails to read is the same kind of answer. +// Today `listProjectState` catches every failure and lists nothing, so a folder whose PROJECT.md +// is a folder, or is unreadable, reads as `no Project State files in this folder` — the one +// answer that says there is nothing to read. The name and the reason belong in the answer: +// `unreadable: PROJECT.md ()`, with the name written the way the other rows write it +// (relative to the working folder, because the answer can be pasted into a public issue). +// +// The control is a folder with no Project State file at all, which must keep answering exactly +// `no Project State files` — an answer that says "nothing here" is wrong only when something is. +import assert from 'node:assert/strict' +import { chmodSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { test } from 'node:test' +import { apply } from '../index.js' +import { fakeContext } from './fake-ctx.js' + +const TEAM = { + version: 1, mainBotIds: ['a'], + bots: { a: { id: 'a', name: 'Chief', role: 'Boss', sessionId: 'a', createdAt: 1 } }, + rooms: {}, agents: {}, exchanges: [], exchangeTurns: {}, questions: {}, prefs: {}, revision: 0, +} + +const NO_FILES = 'project state: no Project State files in this folder' +// The rows this reader answers with one per line; `unreadable` is the one this test is about. +const rows = listing => listing.split('\n').slice(1) +const unreadable = listing => rows(listing).filter(line => line.startsWith('unreadable: ')) + +// A folder the reader calls a repository, with nothing in it that this reader lists. The test +// writes the one state file it wants, so the red case is the only thing in the folder. +function emptyRepository() { + const dir = mkdtempSync(join(tmpdir(), 'ds-bot-project-state-unreadable-')) + mkdirSync(join(dir, '.git')) + return dir +} + +async function boot({ dir } = {}) { + const home = mkdtempSync(join(tmpdir(), 'ds-bot-project-state-unreadable-home-')) + writeFileSync(join(home, 'state.json'), JSON.stringify(TEAM)) + const ctx = fakeContext() + Object.assign(ctx, { + llm: { listProviders: () => [], listModels: async () => [], resolveModelInfo: async () => ({}) }, + workspaceRegistry: { get: () => (dir === undefined ? undefined : { path: dir }), resolveByPath: async () => undefined, create: async path => ({ id: 'w1', path }), archiveSession: async () => {} }, + sessionController: { create: async () => ({ sessionId: 's1' }), resolveAgent: async id => ({ agent: { session: { id }, id, status: 'idle', cancel() {} } }), selectModel: async () => {}, page: async () => ({ records: [], hasMore: false }) }, + sessionTitle: { rename: async () => {} }, + sessions: { flush: async () => {} }, + }) + const rt = apply(ctx, { home }) + await ctx.flybot.ready() + const exec = { agent: { id: 'a', session: { header: { cwd: dir } } } } + const tool = args => ctx.registeredTools.get('project_state').execute(args, exec) + const stop = async () => { await ctx.dispose(); rmSync(home, { recursive: true, force: true, maxRetries: 5 }) } + return { ctx, rt, home, tool, stop } +} + +test('#2461: a Project State name that is a folder is answered as unreadable, never as no files', async (t) => { + const dir = emptyRepository() + mkdirSync(join(dir, 'PROJECT.md')) + const { tool, stop } = await boot({ dir }) + t.after(async () => { await stop(); rmSync(dir, { recursive: true, force: true, maxRetries: 5 }) }) + + const listing = await tool({}) + assert.notEqual(listing, NO_FILES, 'the folder is not empty: one listed name is there and could not be read') + assert.equal(listing.split('\n')[0], 'project state:', 'the answer still opens as the listing it is') + assert.deepEqual(unreadable(listing).map(line => line.replace(/ \([^)]+\)$/, '')), ['unreadable: PROJECT.md'], + 'the name is written the way the rows write it, relative to the working folder') + for (const line of unreadable(listing)) assert.match(line, /^unreadable: PROJECT\.md \([^)\n]+\)$/, + 'the reason travels with the name, so a reader knows it is not an absent file') +}) + +test('#2461: a Project State file that cannot be read keeps its name and reason beside the rows that can', async (t) => { + const dir = emptyRepository() + mkdirSync(join(dir, 'PROJECT.md')) + writeFileSync(join(dir, 'ROADMAP.md'), '# Roadmap\n') + const { tool, stop } = await boot({ dir }) + t.after(async () => { await stop(); rmSync(dir, { recursive: true, force: true, maxRetries: 5 }) }) + + const listing = await tool({}) + const roadmap = rows(listing).find(line => line.startsWith('ROADMAP.md\t')) + assert.ok(roadmap, 'the file that can be read is still listed, with its size, hash and mtime') + assert.equal(roadmap.split('\t').length, 4) + assert.deepEqual(unreadable(listing).length, 1, 'and the one that cannot be read is named once, not swallowed') +}) + +// A root process reads a chmod 000 file, so this case is declared skipped rather than run there — +// the shape `core/test/diagnostics.test.js` already uses for its own root-only cases. +test('#2461: a state file the process may not read answers its code, not an empty folder', { + skip: process.getuid?.() === 0 ? 'root reads a chmod 000 file' : undefined, +}, async (t) => { + const dir = emptyRepository() + writeFileSync(join(dir, 'PROJECT.md'), '# Project\n') + chmodSync(join(dir, 'PROJECT.md'), 0o000) + const { tool, stop } = await boot({ dir }) + t.after(async () => { + await stop() + chmodSync(join(dir, 'PROJECT.md'), 0o600) + rmSync(dir, { recursive: true, force: true, maxRetries: 5 }) + }) + + const listing = await tool({}) + assert.notEqual(listing, NO_FILES, 'an unreadable file is not an absent one') + assert.match(listing, /(^|\n)unreadable: PROJECT\.md \(EACCES\)($|\n)/, + 'the errno is what tells a reader to fix permissions rather than look for another file') +}) + +test('#2461: a folder with no Project State file still answers exactly no files', async (t) => { + const dir = emptyRepository() + const { tool, stop } = await boot({ dir }) + t.after(async () => { await stop(); rmSync(dir, { recursive: true, force: true, maxRetries: 5 }) }) + + assert.equal(await tool({}), NO_FILES) +}) diff --git a/docs/subsystems/core-host.md b/docs/subsystems/core-host.md index 8e69775a8..a19de9741 100644 --- a/docs/subsystems/core-host.md +++ b/docs/subsystems/core-host.md @@ -51,7 +51,7 @@ The server side of the patched `ds-bot` core (`core/`): the part of [ds-bot-core | `core/src/host/usage.js` | Usage ledger at `$DSH_HOME/bot/usage.sqlite` (`calls`). Ledger v3 (#903): every row carries `harness`, `issue_pr`, `tool_calls`, `wall_ms`, plus existing token columns (`withUsageFields` / `missingUsageFields`). Session turns are metered via `foldCalls` over turn events (`tool/call` count, wall time from attempt start, `issuePr` from turn text or `rt.noteUsageIssuePr(sessionId, ref)`), not from llm/stream alone. A triage tier is stamped with `rt.noteUsageHarness(sessionId, idOrLabels)` and lands on the row's `harness` field; the resume cursor (`marks.fold`) stays provider / model / retry. Sample row fixture: `core/test/fixtures/usage-sample.sqlite`. Tests: `core/test/usage-fields.test.js`, `core/test/usage.test.js`. | | `core/src/host/tool-schema.js` | The tool definitions the model sees: its `defineTool` wraps DSH's and completes the compiled schema with `explicitObjectShape`, which fills `properties: {}` and `required: []` on every object node (recursing through `properties`, `items`, `oneOf`/`anyOf`/`allOf`, `$defs`, …). A strict OpenAI-compatible gateway reads an absent member as null and rejects the whole request (`null is not of type "array"`), so one argument-less tool would break every turn; argument validation keeps using DSH's schema unchanged. Every host module registers tools through this wrapper. Tests: `core/test/tool-schema.test.js`. | | `core/src/host/real-dir.js` | The one "what is this directory entry, here" check (#2055), shared by the readers that must not follow a folder link. `dirKind(path)` (async) and `dirKindSync(path)` (sync) answer `real` — the entry itself is a directory and not a symlink; `absent` — nothing at that path (`ENOENT`); or `other` — a symlink, a plain file, a non-directory parent, or any other read error (fail closed). Three states rather than a boolean because a writer has to tell "nothing there, create it" from "a link or a file, refuse"; a reader only tests `=== 'real'`. Both shells share one predicate (`lstat` + `isDirectory()` + `!isSymbolicLink()`), so the rule is written once; `#2056` and `#2057` consume it instead of a fourth hand-rolled copy. The leaf door is the same question asked of a file a writer appends to or truncates ([#2079](https://github.com/ThinkFlowLab/FlyBot/issues/2079)): `fileKind(path)` / `fileKindSync(path)` answer `real` (a plain file carrying one name), `absent` (nothing at that path) or `other` (a link, a hard link whose bytes are another name's, a directory, a non-directory parent, or a check that could not run). Since [#2159](https://github.com/ThinkFlowLab/FlyBot/issues/2159) the leaf door and the temporary name a write lands through share one predicate, so the two answers for the same file cannot disagree (`nlink === 1` in both); the five callers today are all writers — the readers ask `dirKind` — so a hard link refuses the write and never turns a snapshot's file into an unreadable one. Tests: `core/test/real-dir.test.js`. | -| `core/src/host/project-state.js` | The `project_state` tool (#1702, #1550 Stage 2.4): the Project State files of the Session working folder (`PROJECT.md`, `ROADMAP.md`, `docs/governance.md`, `decisions/*.md`, `LESSONS.md`), listed with byte size, sha256 content hash and last-modified time, or one file handed over whole up to 200,000 bytes. A caller compares the hash with the copy it already had, so a stale copy is visible instead of assumed. Answers `absent`, `too large` (with the size) or `not a repository` rather than an empty document, refuses any path outside that list, and reads nothing at install time, no prompt, system message or Session context is written by a call. Since #2037 a decision name has exactly one shape — one path segment under `decisions/`, ending in `.md` — and `isDecisionPath` is the one rule for it: the listing, this reader and the board take a name only when it matches, and both this reader and the board open a file through `describeFile`, which `lstat`s the name and refuses anything that is not a plain file, so a symlink answers `absent` — the shape a missing file already has — instead of the text it points at; since #2041 the bounded byte read is its own export, `readListedFile(dir, path)`, because the board's opened decision reads through it too, so the 200,000-byte bound and the `lstat` rule cannot drift between the two readers ([#2037](https://github.com/ThinkFlowLab/FlyBot/issues/2037), [#2041](https://github.com/ThinkFlowLab/FlyBot/issues/2041)). Since #2055 the `decisions` entry itself must be a directory this reader sees — `dirKind` in `real-dir.js` answers `real` — because `readdir` follows a folder link and the leaf `lstat` sees only the final component: a link at a real folder outside the Project, a link at a missing path and a plain file all read exactly like a folder with no `decisions`, so the listing contributes no row and a read of any `decisions/.md` answers `absent`, never the text it points at ([#2055](https://github.com/ThinkFlowLab/FlyBot/issues/2055)). Tests: `core/test/project-state.test.js`. | +| `core/src/host/project-state.js` | The `project_state` tool (#1702, #1550 Stage 2.4): the Project State files of the Session working folder (`PROJECT.md`, `ROADMAP.md`, `docs/governance.md`, `decisions/*.md`, `LESSONS.md`), listed with byte size, sha256 content hash and last-modified time, or one file handed over whole up to 200,000 bytes. A caller compares the hash with the copy it already had, so a stale copy is visible instead of assumed. Answers `absent`, `too large` (with the size) or `not a repository` rather than an empty document, refuses any path outside that list, and reads nothing at install time, no prompt, system message or Session context is written by a call. Since #2037 a decision name has exactly one shape — one path segment under `decisions/`, ending in `.md` — and `isDecisionPath` is the one rule for it: the listing, this reader and the board take a name only when it matches, and both this reader and the board open a file through `describeFile`, which `lstat`s the name and refuses anything that is not a plain file, so a symlink answers `absent` — the shape a missing file already has — instead of the text it points at; since #2041 the bounded byte read is its own export, `readListedFile(dir, path)`, because the board's opened decision reads through it too, so the 200,000-byte bound and the `lstat` rule cannot drift between the two readers ([#2037](https://github.com/ThinkFlowLab/FlyBot/issues/2037), [#2041](https://github.com/ThinkFlowLab/FlyBot/issues/2041)). Since #2055 the `decisions` entry itself must be a directory this reader sees — `dirKind` in `real-dir.js` answers `real` — because `readdir` follows a folder link and the leaf `lstat` sees only the final component: a link at a real folder outside the Project, a link at a missing path and a plain file all read exactly like a folder with no `decisions`, so the listing contributes no row and a read of any `decisions/.md` answers `absent`, never the text it points at ([#2055](https://github.com/ThinkFlowLab/FlyBot/issues/2055)). Since #2461 a listed name that is there but cannot be read is a row of its own: the listing adds `unreadable: ()` after the readable rows (`` is the errno the read failed with, or `not a plain file` for a folder or other non-file entry), instead of catching every failure into the same silence that answers `no Project State files in this folder`; an absent name, and a symlink, which this reader already answers as absent (#2055), still add nothing, and the readable rows keep their order and their bytes ([#2461](https://github.com/ThinkFlowLab/FlyBot/issues/2461)). Tests: `core/test/project-state.test.js`, `core/test/project-state-unreadable.test.js`. | | `core/src/host/project-assistant.js` | The Project Assistant (#1712, #1550 Stage 2.5): one hidden Bot and Session per Project, created by `project-create` through the roster and refusing an unknown or archived id, with exactly one 09:00 Asia/Shanghai task on that Session through the host's own `schedule-create` path and removed with the Project by `project-delete` — never a cron inside the plugin, and no backfill for a Project made before this slice. Memory lives in `state.projectMemory[id]` (`{decisions, questions, digest}`, behind `rt.projectMemoryOf` / `appendProjectMemory` / `setProjectDigest` and the `project-memory` / `project-memory-write` endpoints) keyed by Project id, so one Project cannot read another's entries. Since #1847 that bound is enforced twice, from one value: `MAX_ENTRY` (600, in `constants.js`) is checked by the assistant writer and again by `appendProjectMemory`, the one write door, and since #1886 both count code points (`codePoints` in `src/host/text.js`, one helper since #1895), not UTF-16 units, so a 600-character emoji note is stored rather than refused as 1,200 units; the endpoint refuses an empty entry (`A Project memory entry needs text`) and one over the cap (`A Project memory entry is at most 600 characters`) instead of storing a blank row or cutting the text, and a refusal leaves the table and `project.updatedAt` untouched; the panel quotes either sentence, in Chinese under a zh locale (`core/src/client/i18n.js`). `buildDigest` answers, in order, what merged since the previous digest, what closed, what is still in progress and on whom, and the next slice, using only board rows (`source`/`column`/`assignee`/`updatedAt`) and the Project State file list; a day with no change is one line, one day has one note and one `deliver()`, and the second build answers `skipped`. Identity is the install's GitHub connector account (the owner's own on a single-person install): with no connector signed in, or on a multi-member host whose `projects.ownerUserId` is not this host's own user, `digest` refuses and escalates instead of posting as the shared app, and every write carries `Written-by: Project Assistant ()` and `Posted-for: `. Since #1733 the memory and the digest are per (Project, owner) — `state.projectMemory[projectId].owners[ownerId]` with the primary owner mirrored on the row, behind `rt.projectMemoryFor(id, ownerId)`, `appendProjectMemory(id, kind, entry)`, `setProjectDigest(id, note, ownerId)` — and one `projectBoard` read plus one `listProjectState` read serve one render per owner, so the facts are shared and the personalisation is not; a read outside `ownersOf(project)` is refused naming that boundary. Its own `schedule-create` sends the assistant's owner as the actor, because a host with a member signed in answers the create with the caller's rows only and the host reads that answer back for the row id. Since #1846 the digest reads the Project's own board, not one member's page: `rt.projectBoard(id, null, { internal: true })` is the one opt-in that skips the per-reader `taskVisible` rule, and the channel builds its options from the zone alone, so no request body can ask for it. Tests: `core/test/project-assistant.test.js`, `core/test/project-digest-authz.test.js`. | | `core/src/host/project-chief.js` | The Project Chief (#1859): every Project gets its own Chief, created with the Project by `project-create` — never the user's main Chief acting part-time. `rt.projectChief` has the same shape as `rt.projectAssistant` (`create(projectId)`, `chiefOf(projectId)`, `remove(projectId)`), the record carries `chief: { botId, sessionId, createdAt }` beside `assistant`, and the spawned Bot is named `Project Chief ()`, bound with `bindProject` so it carries `projectId`, and given instructions that name that one Project and forbid answering for any other (another Project's name is never quoted). A host without the seam logs once and answers `undefined`, so `project-create` still creates the Project with `chief: { error }`; `project-delete` retires the Chief through `deleteBot` and clears its binding. Tests: `core/test/projects.test.js`. | | `core/src/host/project-team.js` | The Project's team dossier (#1864), read-only. `rt.projectTeam(projectId)` answers `{ rows }` in the board's order — the Project's Chief, its assistant, then `roles.maintainers` / `roles.members` — with each row `{ botId, name, role, persona, model, color, createdAt, memory: { head, topics }, skills, rooms }`. `persona` is the Bot's `instructions`; `memory.head` is its own `MEMORY.md` (read through `rt.memoryDigest` in `memory.js`, which also supplies the topic names and never a topic body); `skills` is the attached capabilities the library records as `skill` (a pack contributes its skill children); `rooms` is only the rooms this Project owns. Every string and list is capped (`DOSSIER_LIMITS`: persona and memory head 4000, 64 topics, 64 skills, 32 rooms), a deleted Bot is a `null` row instead of an error for the whole answer, and an unknown Project answers `{ rows: [] }`. The endpoint sits beside `project-read` in `authz.js`'s `TASK_ENDPOINTS`, per the owner decision that persona and memory are shared by the Project's members. Tests: `core/test/project-team.test.js`. | diff --git a/patches/ds-bot/files/src/host/project-state.js b/patches/ds-bot/files/src/host/project-state.js index 19b98c1f9..e3d45d117 100644 --- a/patches/ds-bot/files/src/host/project-state.js +++ b/patches/ds-bot/files/src/host/project-state.js @@ -67,13 +67,20 @@ async function decisionFiles(dir) { // is what `project_state` and the board's opened row already answer for a missing one. async function fileAt(dir, path) { if (isDecisionPath(path) && await dirKind(dirname(join(dir, path))) !== 'real') { - throw new Error(`not a plain file: ${path}`) + throw notPlainFile(path, true) } const info = await lstat(join(dir, path)) - if (!info.isFile()) throw new Error(`not a plain file: ${path}`) + if (!info.isFile()) throw notPlainFile(path, info.isSymbolicLink()) return info } +// A name that is there but is not a plain file. `absent` marks the shapes this reader already +// answers as a missing file — a symlink (#2055) and a decision whose `decisions/` is not a real +// folder here — so the listing drops them exactly like an absent name. Anything else (a folder, a +// fifo, a device) is a name the reader cannot read, which the listing answers instead of dropping +// (#2461): a folder whose `PROJECT.md` is a directory is not a folder with no Project State file. +const notPlainFile = (path, absent) => Object.assign(new Error(`not a plain file: ${path}`), { absent }) + // One Project State file with its byte size, content hash and last-modified time. Exported // because the board reads the file a decided meeting archived, and it must refuse exactly what // this reader refuses — one rule, one place. @@ -90,20 +97,40 @@ export async function describeFile(dir, path) { // The repository gate belongs to the tool, not to every reader (#1809): a decision file is the // board's own artifact and shows up in a folder that is not a repository, while `project_state` // keeps refusing such a folder by default. Only the board passes `{ repository: false }`. +// The reason a listed name has no row, short enough to sit inside one: the system's own errno when +// it gave one, else the rule the name broke (a folder where a plain file belongs). The name itself +// stays in the row, so the reason never repeats it and never carries a parenthesis of its own. +const unreadableReason = (error) => { + if (typeof error?.code === 'string' && error.code !== '') return error.code + const message = String(error?.message ?? error ?? '').split('\n')[0] + return message.startsWith('not a plain file') ? 'not a plain file' : (message.replace(/[()]/g, '') || 'unknown') +} + +// One listed name: a row with its byte size, hash and mtime, or the reason it has none. A name that +// is absent — or is a link, which this reader reads as absent (#2055) — adds nothing; a name that is +// there and could not be read is `unreadable: ()`, never a silent omission (#2461). +async function listedRow(dir, path) { + try { + return { file: await describeFile(dir, path) } + } catch (error) { + if (error?.absent === true || error?.code === 'ENOENT' || error?.code === 'ENOTDIR') return null + return { unreadable: { path, reason: unreadableReason(error) } } + } +} + export async function listProjectState(dir, { repository = true } = {}) { if (repository && !await repositoryAt(dir)) return { ok: false, reason: `not a repository: ${dir}` } const found = [] - for (const path of PROJECT_STATE_PATHS) { - try { - found.push(await describeFile(dir, path)) - } catch { /* an absent Project State file is simply not listed */ } - } - for (const path of await decisionFiles(dir)) { - try { - found.push(await describeFile(dir, path)) - } catch { /* a file that vanished between readdir and read is not listed */ } + const unreadable = [] + const add = async (path) => { + const row = await listedRow(dir, path) + if (row === null) return + if (row.file !== undefined) found.push(row.file) + else unreadable.push(row.unreadable) } - return { ok: true, dir, files: found } + for (const path of PROJECT_STATE_PATHS) await add(path) + for (const path of await decisionFiles(dir)) await add(path) + return { ok: true, dir, files: found, unreadable } } // The bytes of one named Project State file, bounded at `MAX_BYTES`. The path is checked @@ -145,14 +172,21 @@ export function workDirFor(rt, exec) { return rt.teamFolder?.() } -const listed = ({ files }) => files.length === 0 +// The answer the tool prints: the rows a caller compares hashes with, and — after them — the names +// that are there but could not be read. `none` is the answer for a folder with nothing to read, +// which is exactly what an unreadable name is not (#2461). +const listed = ({ files, unreadable }) => files.length === 0 && unreadable.length === 0 ? 'project state: no Project State files in this folder' - : ['project state:', ...files.map(file => `${file.path}\t${file.bytes} bytes\tsha256:${file.sha256}\t${file.mtime}`)].join('\n') + : [ + 'project state:', + ...files.map(file => `${file.path}\t${file.bytes} bytes\tsha256:${file.sha256}\t${file.mtime}`), + ...unreadable.map(({ path, reason }) => `unreadable: ${path} (${reason})`), + ].join('\n') export function install(rt) { rt.ctx.tools.register(defineTool({ name: 'project_state', - description: 'Read the Project State files of your working folder: PROJECT.md, ROADMAP.md, docs/governance.md, decisions/*.md, LESSONS.md. Without `file` it lists each with its byte size, sha256 content hash and last-modified time, so you can compare the hash with the copy you already have. With `file` it returns that one file, bounded. Answers `absent`, `too large` or `not a repository` with the reason instead of an empty document.', + description: 'Read the Project State files of your working folder: PROJECT.md, ROADMAP.md, docs/governance.md, decisions/*.md, LESSONS.md. Without `file` it lists each with its byte size, sha256 content hash and last-modified time, so you can compare the hash with the copy you already have. With `file` it returns that one file, bounded. Answers `absent`, `too large` or `not a repository` with the reason instead of an empty document, and a listed name that is there but cannot be read as `unreadable: ()` rather than leaving it out.', parameters: { file: { type: 'string', description: 'Optional Project State path to read, e.g. decisions/x.md; omit to list them all' }, }, diff --git a/patches/ds-bot/files/test/project-state-unreadable.test.js b/patches/ds-bot/files/test/project-state-unreadable.test.js new file mode 100644 index 000000000..82c9a4382 --- /dev/null +++ b/patches/ds-bot/files/test/project-state-unreadable.test.js @@ -0,0 +1,114 @@ +// A Project State file that is there but cannot be read must not be dropped on the floor. The +// reader already answers `not a repository`, `absent` and `too large` with the reason instead of +// an empty document; a listed name that exists and fails to read is the same kind of answer. +// Today `listProjectState` catches every failure and lists nothing, so a folder whose PROJECT.md +// is a folder, or is unreadable, reads as `no Project State files in this folder` — the one +// answer that says there is nothing to read. The name and the reason belong in the answer: +// `unreadable: PROJECT.md ()`, with the name written the way the other rows write it +// (relative to the working folder, because the answer can be pasted into a public issue). +// +// The control is a folder with no Project State file at all, which must keep answering exactly +// `no Project State files` — an answer that says "nothing here" is wrong only when something is. +import assert from 'node:assert/strict' +import { chmodSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { test } from 'node:test' +import { apply } from '../index.js' +import { fakeContext } from './fake-ctx.js' + +const TEAM = { + version: 1, mainBotIds: ['a'], + bots: { a: { id: 'a', name: 'Chief', role: 'Boss', sessionId: 'a', createdAt: 1 } }, + rooms: {}, agents: {}, exchanges: [], exchangeTurns: {}, questions: {}, prefs: {}, revision: 0, +} + +const NO_FILES = 'project state: no Project State files in this folder' +// The rows this reader answers with one per line; `unreadable` is the one this test is about. +const rows = listing => listing.split('\n').slice(1) +const unreadable = listing => rows(listing).filter(line => line.startsWith('unreadable: ')) + +// A folder the reader calls a repository, with nothing in it that this reader lists. The test +// writes the one state file it wants, so the red case is the only thing in the folder. +function emptyRepository() { + const dir = mkdtempSync(join(tmpdir(), 'ds-bot-project-state-unreadable-')) + mkdirSync(join(dir, '.git')) + return dir +} + +async function boot({ dir } = {}) { + const home = mkdtempSync(join(tmpdir(), 'ds-bot-project-state-unreadable-home-')) + writeFileSync(join(home, 'state.json'), JSON.stringify(TEAM)) + const ctx = fakeContext() + Object.assign(ctx, { + llm: { listProviders: () => [], listModels: async () => [], resolveModelInfo: async () => ({}) }, + workspaceRegistry: { get: () => (dir === undefined ? undefined : { path: dir }), resolveByPath: async () => undefined, create: async path => ({ id: 'w1', path }), archiveSession: async () => {} }, + sessionController: { create: async () => ({ sessionId: 's1' }), resolveAgent: async id => ({ agent: { session: { id }, id, status: 'idle', cancel() {} } }), selectModel: async () => {}, page: async () => ({ records: [], hasMore: false }) }, + sessionTitle: { rename: async () => {} }, + sessions: { flush: async () => {} }, + }) + const rt = apply(ctx, { home }) + await ctx.flybot.ready() + const exec = { agent: { id: 'a', session: { header: { cwd: dir } } } } + const tool = args => ctx.registeredTools.get('project_state').execute(args, exec) + const stop = async () => { await ctx.dispose(); rmSync(home, { recursive: true, force: true, maxRetries: 5 }) } + return { ctx, rt, home, tool, stop } +} + +test('#2461: a Project State name that is a folder is answered as unreadable, never as no files', async (t) => { + const dir = emptyRepository() + mkdirSync(join(dir, 'PROJECT.md')) + const { tool, stop } = await boot({ dir }) + t.after(async () => { await stop(); rmSync(dir, { recursive: true, force: true, maxRetries: 5 }) }) + + const listing = await tool({}) + assert.notEqual(listing, NO_FILES, 'the folder is not empty: one listed name is there and could not be read') + assert.equal(listing.split('\n')[0], 'project state:', 'the answer still opens as the listing it is') + assert.deepEqual(unreadable(listing).map(line => line.replace(/ \([^)]+\)$/, '')), ['unreadable: PROJECT.md'], + 'the name is written the way the rows write it, relative to the working folder') + for (const line of unreadable(listing)) assert.match(line, /^unreadable: PROJECT\.md \([^)\n]+\)$/, + 'the reason travels with the name, so a reader knows it is not an absent file') +}) + +test('#2461: a Project State file that cannot be read keeps its name and reason beside the rows that can', async (t) => { + const dir = emptyRepository() + mkdirSync(join(dir, 'PROJECT.md')) + writeFileSync(join(dir, 'ROADMAP.md'), '# Roadmap\n') + const { tool, stop } = await boot({ dir }) + t.after(async () => { await stop(); rmSync(dir, { recursive: true, force: true, maxRetries: 5 }) }) + + const listing = await tool({}) + const roadmap = rows(listing).find(line => line.startsWith('ROADMAP.md\t')) + assert.ok(roadmap, 'the file that can be read is still listed, with its size, hash and mtime') + assert.equal(roadmap.split('\t').length, 4) + assert.deepEqual(unreadable(listing).length, 1, 'and the one that cannot be read is named once, not swallowed') +}) + +// A root process reads a chmod 000 file, so this case is declared skipped rather than run there — +// the shape `core/test/diagnostics.test.js` already uses for its own root-only cases. +test('#2461: a state file the process may not read answers its code, not an empty folder', { + skip: process.getuid?.() === 0 ? 'root reads a chmod 000 file' : undefined, +}, async (t) => { + const dir = emptyRepository() + writeFileSync(join(dir, 'PROJECT.md'), '# Project\n') + chmodSync(join(dir, 'PROJECT.md'), 0o000) + const { tool, stop } = await boot({ dir }) + t.after(async () => { + await stop() + chmodSync(join(dir, 'PROJECT.md'), 0o600) + rmSync(dir, { recursive: true, force: true, maxRetries: 5 }) + }) + + const listing = await tool({}) + assert.notEqual(listing, NO_FILES, 'an unreadable file is not an absent one') + assert.match(listing, /(^|\n)unreadable: PROJECT\.md \(EACCES\)($|\n)/, + 'the errno is what tells a reader to fix permissions rather than look for another file') +}) + +test('#2461: a folder with no Project State file still answers exactly no files', async (t) => { + const dir = emptyRepository() + const { tool, stop } = await boot({ dir }) + t.after(async () => { await stop(); rmSync(dir, { recursive: true, force: true, maxRetries: 5 }) }) + + assert.equal(await tool({}), NO_FILES) +})