From 5d1483ca21c4a3a02207d143cf7e87f8d90e3492 Mon Sep 17 00:00:00 2001 From: Danyang Chen Date: Mon, 26 Oct 2026 15:12:25 +0800 Subject: [PATCH] fix(delivery): read the setting a user wrote, and resolve a worktree's origin (#1258) The hint, the page and the docs all tell an owner to set `delivery.repo`, but the host expands no dotted key and only `config.deliveryRepo` was read, so the documented setting was silently ignored and the board fell through to the package default. Read the three shapes a profile can carry, first one defined wins; validation stays in `parseRepoName`. A linked worktree keeps no `config` in the gitdir its `.git` points at, only a `commondir` naming the shared directory that does, so a worktree workspace also fell through to the package default. Follow the pointer, and keep the submodule layout where `config` is there. The old fixture built a worktree's gitdir with a `config` in it, which is the submodule shape; both layouts now have their own test named after the layout it builds. `OWNER_NAME` allowed a dot anywhere, so `../..` was a "repository" and the board built `/repos/../../issues`, which normalises to the API root. Refuse a leading dot in either segment. Same host, so no token left, but the name invariant was gone. --- changelog.d/1258-delivery-repo.fixed.md | 1 + core/src/host/delivery-board.js | 4 +- core/src/host/delivery-repo.js | 52 ++++++++++++-- core/test/delivery-board.test.js | 14 ++++ core/test/delivery-repo.test.js | 72 +++++++++++++++++-- docs/subsystems/core-host.md | 4 +- patches/ds-bot/0005-social-seams.patch | 39 +++++----- patches/ds-bot/0012-room-membership.patch | 65 +++++++++-------- .../ds-bot/files/src/host/delivery-board.js | 4 +- .../ds-bot/files/src/host/delivery-repo.js | 52 ++++++++++++-- .../ds-bot/files/test/delivery-board.test.js | 14 ++++ .../ds-bot/files/test/delivery-repo.test.js | 72 +++++++++++++++++-- 12 files changed, 306 insertions(+), 87 deletions(-) create mode 100644 changelog.d/1258-delivery-repo.fixed.md diff --git a/changelog.d/1258-delivery-repo.fixed.md b/changelog.d/1258-delivery-repo.fixed.md new file mode 100644 index 000000000..f4e1425d5 --- /dev/null +++ b/changelog.d/1258-delivery-repo.fixed.md @@ -0,0 +1 @@ +The delivery board reads `delivery.repo` however the instance wrote it — nested (`delivery: { repo: … }`), as a literal dotted key, or flat (`deliveryRepo`) — instead of only the flat one, so the setting the page and the hint name really names the repository ([#1258](https://github.com/ThinkFlowLab/FlyBot/issues/1258)). A workspace that is a linked `git worktree` now resolves its `origin` through the worktree's `commondir` rather than falling back to the package default, and a repository name with a `.` or `..` segment is refused instead of being turned into `/repos/../../issues`. diff --git a/core/src/host/delivery-board.js b/core/src/host/delivery-board.js index e3b47a046..a3ae1791a 100644 --- a/core/src/host/delivery-board.js +++ b/core/src/host/delivery-board.js @@ -11,7 +11,7 @@ // `{ message, endpoint, status, kind, scope, at }` so the view can say what failed, where, and what // to do about it. "No GitHub connection yet" is reserved for a host that has never tried. import { repoSlug } from './update.js' -import { NO_REPO_MESSAGE, resolveRepo } from './delivery-repo.js' +import { NO_REPO_MESSAGE, instanceRepoSetting, resolveRepo } from './delivery-repo.js' export const DELIVERY_COLUMNS = ['raw', 'ready', 'working', 'review', 'done-today'] @@ -761,7 +761,7 @@ export function install(rt) { try { return rt.ctx?.workspaceRegistry?.get?.(rt.state?.workspaceId)?.path ?? null } catch { return null } } const setting = () => { - try { return rt.config?.deliveryRepo ?? null } catch { return null } + try { return instanceRepoSetting(rt.config) } catch { return null } } // Level 3 is the package's own repository, not the instance's choice. The shipped package carries // it; a host whose package.json has none (or a test) hands in '' and the board refuses rather than diff --git a/core/src/host/delivery-repo.js b/core/src/host/delivery-repo.js index 62944dab6..16ef630e2 100644 --- a/core/src/host/delivery-repo.js +++ b/core/src/host/delivery-repo.js @@ -5,12 +5,14 @@ // were. One repository is resolved here, for the host and for `scripts/delivery-probe.mjs` alike, // in three steps, first answer wins: // -// 1. the instance setting, `delivery.repo` on the plugin entry (`config.deliveryRepo`); +// 1. the instance setting, `delivery.repo` on the plugin entry — the host hands the entry through +// verbatim, so all three spellings people write are read (`instanceRepoSetting`); // 2. the workspace's own git remote — `/.git/config`, `[remote "origin"] url`; // 3. the package's `repository.url`, which is a default and is labelled as one. // -// Nothing is spawned: the host may run where `git` is not on PATH. A `.git` that is a file (a -// worktree or a submodule) is followed through its `gitdir:` pointer instead. A name is normalised +// Nothing is spawned: the host may run where `git` is not on PATH. A `.git` that is a file is +// followed through its `gitdir:` pointer: a submodule keeps `config` there, and a linked worktree +// keeps only a `commondir` pointer to the shared directory that does (#1258). A name is normalised // to `owner/name`; a GitHub URL or the scp form `git@github.com:owner/name.git` is accepted where a // name is, because that is what people paste. import { readFileSync, statSync } from 'node:fs' @@ -19,7 +21,10 @@ import { join, resolve } from 'node:path' // The wording acceptance criterion 4 asks for, in one place so the view, the host and the CLI agree. export const NO_REPO_MESSAGE = 'no repository — set delivery.repo' -const OWNER_NAME = /^[\w.-]+\/[\w.-]+$/ +// A segment is a plain repository name part: word characters, dots and dashes, but never a leading +// dot. `.` and `..` are path steps, not names, and a board that accepted them would build +// `/repos/../../issues` and normalise it to the API root (#1258). +const OWNER_NAME = /^(?!\.)[\w.-]+\/(?!\.)[\w.-]+$/ const SCP_GITHUB = /^(?:[\w.-]+@)?github\.com:([\w.-]+\/[\w.-]+?)(?:\.git)?\/?$/i const cleanName = text => { @@ -61,18 +66,51 @@ export function originUrlIn(text) { return null } -// Where git keeps the config for this folder: `.git/config`, or the `gitdir:` a `.git` file names. -// Null when the folder is not a git checkout at all. +// Where git keeps the config for this folder: `.git/config`, or whatever a `.git` file's `gitdir:` +// pointer leads to. A submodule keeps `config` in that directory; a linked worktree keeps only a +// `commondir` file naming the shared directory that has it (#1258). Null when the folder is not a +// git checkout at all. export function gitConfigPath(folder) { if (typeof folder !== 'string' || folder === '') return null const dot = join(folder, '.git') try { if (statSync(dot).isDirectory()) return join(dot, 'config') const pointer = /^gitdir:\s*(.+)$/m.exec(readFileSync(dot, 'utf8')) - return pointer ? join(resolve(folder, pointer[1].trim()), 'config') : null + if (!pointer) return null + const gitdir = resolve(folder, pointer[1].trim()) + const own = join(gitdir, 'config') + if (isFile(own)) return own + const common = commonDirOf(gitdir) + return common === null ? null : join(common, 'config') } catch { return null } } +const isFile = path => { + try { return statSync(path).isFile() } catch { return false } +} + +// A linked worktree's gitdir holds no config: `commondir` names the common directory, relative to +// the gitdir, where the shared `config` lives. +const commonDirOf = gitdir => { + try { + const pointer = readFileSync(join(gitdir, 'commondir'), 'utf8').trim() + return pointer === '' ? null : resolve(gitdir, pointer) + } catch { return null } +} + +// The instance setting as the profile actually wrote it. The host passes the plugin entry through +// verbatim and expands no dotted key, so one setting reaches us as a flat `deliveryRepo`, a literal +// `'delivery.repo'`, or a nested `delivery.repo` — the hint, the page and the docs all say the last +// spelling, and reading only the first would ignore what a user just wrote (#1258). First defined +// answer wins; validation stays in `parseRepoName`. +export function instanceRepoSetting(config) { + if (config === null || typeof config !== 'object') return null + for (const value of [config.delivery?.repo, config['delivery.repo'], config.deliveryRepo]) { + if (value !== undefined && value !== null && value !== '') return value + } + return null +} + // The `owner/name` the folder's `origin` names, read from disk without running git. export function gitOriginRepo(folder) { const path = gitConfigPath(folder) diff --git a/core/test/delivery-board.test.js b/core/test/delivery-board.test.js index 3a29d288c..bd0daa8f6 100644 --- a/core/test/delivery-board.test.js +++ b/core/test/delivery-board.test.js @@ -852,6 +852,20 @@ test('the instance setting names the repository for the board and the probe, and assert.ok(probe.rows.every(row => row.ok)) }) +test('a dotted or nested delivery.repo names the repository too, as a user writes it (#1258)', async () => { + // The hint says `delivery.repo`, so that is what a profile carries — as a literal dotted key or + // nested; a literal key wins nowhere, it is simply read. The flat `deliveryRepo` is the third. + for (const config of [{ 'delivery.repo': 'acme/widget' }, { delivery: { repo: 'acme/widget' } }]) { + const github = fakeGitHub({ slug: 'acme/widget' }) + const installed = hostWith(github, { config }) + const settled = await installed.deliveryBoard({ wait: true }) + assert.equal(settled.repo, 'acme/widget', JSON.stringify(config)) + assert.equal(settled.repoSource, 'config', JSON.stringify(config)) + assert.equal(settled.state, 'fresh') + for (const call of github.calls) assert.match(call.url, /^https:\/\/api\.github\.com\/repos\/acme\/widget(\/|$|\?)/, call.url) + } +}) + test('the workspace git remote names the repository when there is no setting (#1250)', async () => { const dir = mkdtempSync(join(tmpdir(), 'delivery-install-')) try { diff --git a/core/test/delivery-repo.test.js b/core/test/delivery-repo.test.js index d25659333..5cb2b59d0 100644 --- a/core/test/delivery-repo.test.js +++ b/core/test/delivery-repo.test.js @@ -7,7 +7,7 @@ import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'nod import { tmpdir } from 'node:os' import { join } from 'node:path' import { test } from 'node:test' -import { gitConfigPath, gitOriginRepo, NO_REPO_MESSAGE, originUrlIn, parseRepoName, resolveRepo } from '../src/host/delivery-repo.js' +import { gitConfigPath, gitOriginRepo, instanceRepoSetting, NO_REPO_MESSAGE, originUrlIn, parseRepoName, resolveRepo } from '../src/host/delivery-repo.js' const scratch = () => mkdtempSync(join(tmpdir(), 'delivery-repo-')) const cleanup = dir => rmSync(dir, { recursive: true, force: true }) @@ -56,18 +56,43 @@ test('an origin written as a URL, an scp path or a name with no .git reads the s } }) -test('a .git file is followed through its gitdir, and git is never spawned (#1250)', () => { - // A worktree (or a submodule) keeps its config elsewhere and `.git` is a pointer file. +test('a submodule keeps its config in the gitdir the `.git` file names (#1250)', () => { + // A submodule's `.git` is a pointer file and its own gitdir really does hold `config`. const root = scratch() try { - const store = join(root, 'main.git', 'worktrees', 'wt') + const store = join(root, 'super', '.git', 'modules', 'sub') mkdirSync(store, { recursive: true }) - writeFileSync(join(store, 'config'), '[remote "origin"]\n\turl = git@github.com:acme/worktree.git\n') + writeFileSync(join(store, 'config'), '[remote "origin"]\n\turl = git@github.com:acme/submodule.git\n') + const dir = join(root, 'super', 'sub') + mkdirSync(dir, { recursive: true }) + writeFileSync(join(dir, '.git'), `gitdir: ${join('..', '.git', 'modules', 'sub')}\n`) + assert.equal(gitConfigPath(dir), join(store, 'config')) + assert.deepEqual(resolveRepo({ workspace: dir }), { repo: 'acme/submodule', source: 'remote' }) + } finally { cleanup(root) } +}) + +test('a linked worktree follows its `commondir` to the config the shared directory holds (#1258)', () => { + // `git worktree add` writes `.git` -> `
/.git/worktrees/`, and that directory holds no + // config of its own: only `commondir`, naming the shared directory where git keeps it. Reading + // `config` beside the pointer finds nothing, so a worktree workspace fell to the package default. + const root = scratch() + try { + const shared = join(root, 'main', '.git') + mkdirSync(join(shared, 'worktrees', 'wt'), { recursive: true }) + writeFileSync(join(shared, 'config'), '[remote "origin"]\n\turl = git@github.com:acme/worktree.git\n') + writeFileSync(join(shared, 'worktrees', 'wt', 'commondir'), '../..\n') const dir = join(root, 'checkout') mkdirSync(dir) - writeFileSync(join(dir, '.git'), `gitdir: ${join('..', 'main.git', 'worktrees', 'wt')}\n`) - assert.equal(gitConfigPath(dir), join(store, 'config')) + writeFileSync(join(dir, '.git'), `gitdir: ${join('..', 'main', '.git', 'worktrees', 'wt')}\n`) + assert.equal(gitConfigPath(dir), join(shared, 'config')) assert.deepEqual(resolveRepo({ workspace: dir }), { repo: 'acme/worktree', source: 'remote' }) + // A pointer with neither a config nor a commondir answers nothing rather than a wrong path. + const bare = join(root, 'bare') + mkdirSync(join(bare, 'gitdir'), { recursive: true }) + mkdirSync(join(root, 'elsewhere')) + writeFileSync(join(root, 'elsewhere', '.git'), 'gitdir: ../bare/gitdir\n') + assert.equal(gitConfigPath(join(root, 'elsewhere')), null) + assert.deepEqual(resolveRepo({ workspace: join(root, 'elsewhere') }), { repo: null, source: null }) } finally { cleanup(root) } }) @@ -108,6 +133,39 @@ test('a value that is not a GitHub repository is refused, not mined for its last assert.equal(originUrlIn('[remote "origin"]\n\tfetch = +refs/heads/*:refs/remotes/origin/*\n'), null) }) +test('a path step is not a repository name, so `.` and `..` segments are refused (#1258)', () => { + // `OWNER_NAME` allowed a dot anywhere, so `../..` answered a "repository": the board then built + // `/repos/../../issues`, which normalises to the API root. Same host, so no token left, but every + // request it made was meaningless and the "this is a repository name" invariant was gone. + for (const value of ['../..', './widget', '.a/b', 'acme/.b', 'acme/..']) { + assert.equal(parseRepoName(value), null, value) + } + // A dot inside a segment is a real repository name and stays allowed. + for (const value of ['acme/widget.js', 'a.b/c-d', 'a1-2_3/b.4']) { + assert.equal(parseRepoName(value), value, value) + } +}) + +test('the instance setting is read in all three spellings a profile can write it (#1258)', () => { + // The host passes the plugin entry through verbatim and expands no dotted key, so the hint's + // `delivery.repo` reaches the board as one of three shapes. Reading only the flat one ignored + // what a user had just written and fell through to the labelled package default. + assert.equal(instanceRepoSetting({ delivery: { repo: 'acme/widget' } }), 'acme/widget') + assert.equal(instanceRepoSetting({ 'delivery.repo': 'acme/widget' }), 'acme/widget') + assert.equal(instanceRepoSetting({ deliveryRepo: 'acme/widget' }), 'acme/widget') + // The documented spelling wins over the runtime shape when a profile happens to carry both. + assert.equal(instanceRepoSetting({ delivery: { repo: 'acme/nested' }, deliveryRepo: 'acme/flat' }), 'acme/nested') + // Nothing written, or an empty value, is "no setting", not the empty string as a repository. + assert.equal(instanceRepoSetting({}), null) + assert.equal(instanceRepoSetting({ delivery: {} }), null) + assert.equal(instanceRepoSetting({ deliveryRepo: '' }), null) + assert.equal(instanceRepoSetting(null), null) + assert.equal(instanceRepoSetting(undefined), null) + // A nested setting is a real setting end to end, not only in the helper. + assert.deepEqual(resolveRepo({ setting: instanceRepoSetting({ delivery: { repo: 'acme/widget' } }), packageSlug: 'x/y' }), + { repo: 'acme/widget', source: 'config' }) +}) + test('the probe is a shell over this resolver, and refuses before it asks (#1250)', () => { const source = readFileSync(new URL('../scripts/delivery-probe.mjs', import.meta.url), 'utf8') // One resolver, imported — a second copy in the CLI is how the two drift. diff --git a/docs/subsystems/core-host.md b/docs/subsystems/core-host.md index 20716ac59..a292e4528 100644 --- a/docs/subsystems/core-host.md +++ b/docs/subsystems/core-host.md @@ -38,14 +38,14 @@ The server side of the patched `ds-bot` core (`core/`): the part of [ds-bot-core | --- | --- | | `core/index.js` | Server entry. | | `core/src/host/` | Host-side helpers (non-hooks). | -| `core/src/host/delivery-board.js` | The delivery board (#1173): one read-only snapshot of the repository's GitHub issues, pull requests and CI runs, laid out by delivery stage. Every request is a `GET`; the token is read host-side (`rt.secretValue`, falling back to none) and never leaves the payload, which carries no credential. `read({ force })` caches per URL with the ETag and revalidates with `If-None-Match`, serving the old snapshot on a 304 and while a refresh runs; there is no timer — the client asks. Issues are placed by label (`agent-working` → working, `agent-ready` → ready, else original), pull requests in review, and pull requests merged, and issues or pull requests closed, since the day start in done today — a pull request closed without merging counts, so finished work never sits in no column. A PR card carries the verdict badge of a recent `Verdict:` on its own tip (an old-tip verdict does not count; CI decides) — every comment page is read, so a verdict past the first hundred comments still decides — and an issue is linked to a pull request only when a closing keyword (`close`/`fix`/`resolve` with `#n`) names it, never because a body merely mentions the number (#1189), and the header reports the Asia/Shanghai day, the 7-day window and CI as `counted` of `total`. The payload also carries `metrics.daily` (#1222): the last seven Asia/Shanghai days, oldest first and today last with `partial: true`, each with the issues and pull requests opened, closed and merged and one `{ failed, counted, total }` CI block per day — a cancelled run is left out of the denominator. A day whose list or runs were truncated, or whose CI could not be read, is `null` and never `0`; a week that could not be read makes the whole series `null`, so the view says it has no data instead of drawing seven zeros. The series costs no request: it is assembled from the week and today windows the round already reads. Endpoint `delivery-board`, allowed to any signed-in member because it reads one repository rather than an owner's Bots. #1211: a read never reports a blank board for a problem it can name. The first read answers `{ state: 'loading', since, attempt }` at once and the round runs in the background on the same `inflight`, because a request parked on a whole round is cut by the reverse proxy before it answers. Then `fresh`, `stale` (an older snapshot kept, with `error: { message, endpoint, status, at }` carried into the next read rather than dropped to `null`) and `partial` (a round past `BUILD_BUDGET_MS` — 90 s — lands with the columns it has and the metrics it read). Degradation comes before retry: an unreadable `/actions/runs` makes CI `null` and the row says `unknown`, never `0`; an unreadable check-runs writes `checks-unknown`; unreadable comments leave that card's owner blank; only the two open tables (`pulls`, `issues`) fail a round, because without them there are no columns. A failed round backs off `10 s → 30 s → 2 min` (`BACKOFF_MS`) unless the client forces a read, and a 403 names `retry-after`, else `max(0, x-ratelimit-reset - now)`, and no number at all when neither is present. `probeGitHub()` walks five endpoints and returns status plus the setting to fix; `probe()` and the `delivery-probe` endpoint expose it, and `core/scripts/delivery-probe.mjs` is a thin shell over the same function. Since #1250 the repository is not baked in: every read resolves `{ repo, source }` through `delivery-repo.js` — the instance setting `delivery.repo` (`config.deliveryRepo`), then the workspace's `.git/config` `[remote "origin"] url`, then the package's own `repository.url` as a labelled default — and the payload carries `repoSource` (`config` / `remote` / `package`) for the header. One board is kept per repository, so a changed setting starts its new repository at `loading`; nothing resolving answers a failed read with `no repository — set delivery.repo` before any board exists, so no request goes to `/repos/null`. Tests: `core/test/delivery-board.test.js`, `core/test/delivery-repo.test.js`. | +| `core/src/host/delivery-board.js` | The delivery board (#1173): one read-only snapshot of the repository's GitHub issues, pull requests and CI runs, laid out by delivery stage. Every request is a `GET`; the token is read host-side (`rt.secretValue`, falling back to none) and never leaves the payload, which carries no credential. `read({ force })` caches per URL with the ETag and revalidates with `If-None-Match`, serving the old snapshot on a 304 and while a refresh runs; there is no timer — the client asks. Issues are placed by label (`agent-working` → working, `agent-ready` → ready, else original), pull requests in review, and pull requests merged, and issues or pull requests closed, since the day start in done today — a pull request closed without merging counts, so finished work never sits in no column. A PR card carries the verdict badge of a recent `Verdict:` on its own tip (an old-tip verdict does not count; CI decides) — every comment page is read, so a verdict past the first hundred comments still decides — and an issue is linked to a pull request only when a closing keyword (`close`/`fix`/`resolve` with `#n`) names it, never because a body merely mentions the number (#1189), and the header reports the Asia/Shanghai day, the 7-day window and CI as `counted` of `total`. The payload also carries `metrics.daily` (#1222): the last seven Asia/Shanghai days, oldest first and today last with `partial: true`, each with the issues and pull requests opened, closed and merged and one `{ failed, counted, total }` CI block per day — a cancelled run is left out of the denominator. A day whose list or runs were truncated, or whose CI could not be read, is `null` and never `0`; a week that could not be read makes the whole series `null`, so the view says it has no data instead of drawing seven zeros. The series costs no request: it is assembled from the week and today windows the round already reads. Endpoint `delivery-board`, allowed to any signed-in member because it reads one repository rather than an owner's Bots. #1211: a read never reports a blank board for a problem it can name. The first read answers `{ state: 'loading', since, attempt }` at once and the round runs in the background on the same `inflight`, because a request parked on a whole round is cut by the reverse proxy before it answers. Then `fresh`, `stale` (an older snapshot kept, with `error: { message, endpoint, status, at }` carried into the next read rather than dropped to `null`) and `partial` (a round past `BUILD_BUDGET_MS` — 90 s — lands with the columns it has and the metrics it read). Degradation comes before retry: an unreadable `/actions/runs` makes CI `null` and the row says `unknown`, never `0`; an unreadable check-runs writes `checks-unknown`; unreadable comments leave that card's owner blank; only the two open tables (`pulls`, `issues`) fail a round, because without them there are no columns. A failed round backs off `10 s → 30 s → 2 min` (`BACKOFF_MS`) unless the client forces a read, and a 403 names `retry-after`, else `max(0, x-ratelimit-reset - now)`, and no number at all when neither is present. `probeGitHub()` walks five endpoints and returns status plus the setting to fix; `probe()` and the `delivery-probe` endpoint expose it, and `core/scripts/delivery-probe.mjs` is a thin shell over the same function. Since #1250 the repository is not baked in: every read resolves `{ repo, source }` through `delivery-repo.js` — the instance setting `delivery.repo` (read as a nested, a dotted or a flat key, since the host expands no dotted name), then the workspace's `.git/config` `[remote "origin"] url`, then the package's own `repository.url` as a labelled default — and the payload carries `repoSource` (`config` / `remote` / `package`) for the header. One board is kept per repository, so a changed setting starts its new repository at `loading`; nothing resolving answers a failed read with `no repository — set delivery.repo` before any board exists, so no request goes to `/repos/null`. Tests: `core/test/delivery-board.test.js`, `core/test/delivery-repo.test.js`. | | `core/src/host/prompt.js` | The Bot system prompt; a Main Bot's section is headed `You are the Main Bot` and uses that one term, the one the UI shows (#276). The team rules state the idle default (#699): a Bot does nothing until the user addresses it, it is @-mentioned in a group, its parent gives it a task, or unfinished tasks it was already given are still on its board. It does not invent work. A coordinator acts on a user task rather than on its own initiative — an addressed Bot answers as before. | | `core/src/host/room-membership.js` | Typed room participants (human / local bot / granted remote), authorship, membership-window history (#50); rooms with friends' Bots or other owners take granted local Bots only and evict a Bot that stops qualifying (#90). Face-to-face rooms (#83) reuse this model (`origin: 'face-to-face'`, room-only remotes without a person-grant, `upsertFaceRoom` / `syncFaceParticipants` / `applyFaceRoomLine`); a face-room local Bot is `viaCode` (brought by number), `evictUnshared` skips face rooms (membership is the server's), and a Bot that returns to the member list drops the empty local `left` line (#236). Refusing an ungranted local Bot next to a friend's Bot throws `ungrantedRefusal`: a message that names the Bot, the friend and the two ways out, plus `error.details` `{ reason: 'ungranted-local', bots, remotes }`; `channel.js` `fail` forwards a plain-object `details` (empty otherwise) and `client/api.js` puts it on the thrown error, where `client/new-chat.js` draws a card (**Copy invite link for {Bot}** via `__flybot__.social.createInvite`, **Leave {Bot} out**) read through `ungrantedLocalFix` in `shared/remote-label.js`; the same file's `copiedToClipboard` reports whether the invite link really reached the clipboard (none, or a refused write, is `false`), and the card then shows the link to copy by hand instead of saying it was copied. | | `core/src/host/face-mentions.js` | Who "@" calls in a face-to-face room (#287): every alias of a participant (`faceAliases`, kept in `shared/remote-label.js` so the bubble chips the same names, #305; bare name, the picker label `'s Chief` / ` · 's Bot`, ` · `, handle), local and friend's Bots told apart. `faceMentions().handles` is the handle of every Bot a line names (the explicit mention list a sender attaches, #293) and `mentionedByHandle` resolves a received list to my Bots (`known` counts handles the room has). | | `core/src/host/reach.js` | Per-Bot grants and the owner-policy `reach()` check that hooks.js runs outside the plugin chain (#90); `rt.setGrants`, `rt.hasGrant`, `rt.hasAnyGrant`. A local Bot refused toward a friend's Bot is told that it is not shared with that friend and where to share it (Sharing, #273). `rt.reach` also reads the room a send already belongs to when the caller names none (#948): `sharedRoom` applies only to the `roomOnly` identity — the Bot holds no grant at all and came into that **face-to-face** room by its number (`isCodeJoined`) — and looks for the one such room where the **exact** target is still seated; a Bot that holds a grant, or a room of this home, is judged as it always was and keeps asking for an explicit `roomId`, so a Bot that joined a room by number — a `roomOnly` identity with no person-grant — may line that seat, which is the same allowance `reach()` already applies to an explicit `roomId`; the room itself is named outbound by the transport, never assumed here. A `user-mention` with no named room is also judged under `mentionRoom` (#1126): one live local or inbound group where that Bot is a member and the exact target is still a remote seat, including a seat a group invite admitted (`admittedBy: 'ticket'`). A plain line, `message_bot`, a seat that has left, and a stranger outside every such room stay on the grant rule. Face-to-face stays on `sharedRoom`. Tests: `core/test/reach-policy.test.js`, `core/test/flybot.test.js`. | | `core/src/host/schedules.js` | Scheduled tasks through DSH's schedule service: list, create, edit, pause/resume, retry and delete for Settings → Scheduled tasks, each task's last run, and moving a Bot's tasks to its fresh part (#354, #473, see **Scheduled tasks: edit, pause, missed runs** and **Scheduled tasks: last run and Retry**). Also the `bot_schedules` tool: the Bot-wide list DSH's own `schedule_list` cannot give, group chats' own Sessions included (#628, see **Scheduled tasks: the Bot-wide list**). Tests: `core/test/schedules.test.js`. Tests for team quiet hours, its refusals and the settings write: `core/test/quiet-hours.test.js`; the rule itself is in `core/src/shared/quiet-hours.js` (#1166). | | `core/src/host/automation-default.js` | Automation tasks on by default (#378): once per profile, `offerBundle` asks DSH's plugin manager to switch on `@deepseek-ai/dsh-experimental-schedule-bundle` and writes the offer to `dsh-defaults.json` next to the team. Tests: `core/test/automation-default.test.js`. | -| `core/src/host/delivery-repo.js` | Which repository a delivery read talks to, and where the name came from (#1250). `resolveRepo({ setting, workspace, packageSlug })` answers first-wins: the instance setting, then the team workspace's `origin` read straight out of `.git/config` (a `.git` file is followed through its `gitdir:`, and no `git` process is spawned), then the package's `repository.url`. A name is normalised to `owner/name` from `owner/name`, a GitHub URL, `git@github.com:owner/name.git` or an `ssh://` URL; anything on another host is refused rather than mined. Only `origin` counts — a union of disagreeing remotes would be a guess. `NO_REPO_MESSAGE` is the one wording the host, the view and `core/scripts/delivery-probe.mjs` use. Tests: `core/test/delivery-repo.test.js` (the knife for each level). | +| `core/src/host/delivery-repo.js` | Which repository a delivery read talks to, and where the name came from (#1250). `resolveRepo({ setting, workspace, packageSlug })` answers first-wins: the instance setting, then the team workspace's `origin` read straight out of `.git/config` (a `.git` file is followed through its `gitdir:`; a linked worktree then follows its `commondir` to the shared config, and no `git` process is spawned), then the package's `repository.url`. A name is normalised to `owner/name` from `owner/name`, a GitHub URL, `git@github.com:owner/name.git` or an `ssh://` URL; anything on another host is refused rather than mined, and a `.` or `..` segment is refused because it is a path step rather than a repository name (#1258). Only `origin` counts — a union of disagreeing remotes would be a guess. `NO_REPO_MESSAGE` is the one wording the host, the view and `core/scripts/delivery-probe.mjs` use. Tests: `core/test/delivery-repo.test.js` (the knife for each level). | | `core/src/host/notify.js` | Out-of-app delivery, first half (#355): a transport-agnostic adapter with Slack's `chat.postMessage` shape, one key per reminder so a retried send cannot duplicate, exponential backoff that honours `Retry-After`, three attempts at most, a structural failure not retried, and zero transport calls while the channel is off. Ships `fakeSlack` for tests; nothing calls it until the real Slack transport lands (see **Out-of-app delivery, first half**). Tests: `core/test/notify.test.js`. | | `core/src/host/harness.js` | Declarative harness (#874 Option B). Step 0 (#903): `default` names today's prompt / tools / model / step·retry / verifiers (`rt.harness`, `rt.resolveHarness`); behavior-neutral. Step 1: `lite` / `medium` / `heavy` load through the same resolver with distinct declared budgets; `rt.routeHarness` reads one triage label (`harness:lite`, `harness:medium`, `harness:heavy`). `applied` stays false, so the named modules are not rewritten. `harnessPro` checks a config offline against CI, `core/dev/eval`, and `rsi-baseline` and does not call a model. Step 2: `rt.harnessExperience` reads the experience library and does not attach it to a tier. Tree prompt exception: `@flybot/tree` keeps its marked prompt under `plugins/tree` (outside the three mirrors); listed on the harness, injection stays outside this file. Installed in `MODULES` right after `store`. Tests: `core/test/harness.test.js`, `core/test/experience.test.js`. | | `core/src/host/experience.js` | Experience library read API (#874 Step 2). `lookupExperience({ repo, techStack, query, root })` reads `core/dev/experience/`. Data and `check.mjs` stay under `core/dev/` (see [ds-bot-core](ds-bot-core.md)). | diff --git a/patches/ds-bot/0005-social-seams.patch b/patches/ds-bot/0005-social-seams.patch index ceba0af2b..9834a17a8 100644 --- a/patches/ds-bot/0005-social-seams.patch +++ b/patches/ds-bot/0005-social-seams.patch @@ -288,7 +288,7 @@ diff --git a/src/host/channel.js b/src/host/channel.js const invalid = message => Object.assign(new Error(message), { code: 'bot/invalid' }) // Programming and file system errors are DS Bot's own faults, logged with their stack; -@@ -21,32 +21,146 @@ +@@ -21,32 +21,146 @@ export function install(rt) { // The DSH interface language of the last browser that asked for the team. let uiLocale @@ -444,7 +444,7 @@ diff --git a/src/host/channel.js b/src/host/channel.js // The team's DSH Agent (at most one): plain dsh Sessions, not Bots. agents: Object.values(state.agents), // Key names and who may use them; values never leave the host. -@@ -54,7 +54,8 @@ +@@ -54,7 +54,8 @@ export function install(rt) { capabilities: rt.capabilityLibraryView(), secretRequests: state.secretRequests ?? {}, colors: [...COLORS, ...EXTRA_COLORS], @@ -454,7 +454,7 @@ diff --git a/src/host/channel.js b/src/host/channel.js defaultModel: rt.cachedDefault() ?? null, permissions: rt.permissionCatalog?.() ?? null, modelsKey: modelsKey(), -@@ -65,22 +65,51 @@ +@@ -65,22 +65,51 @@ export function install(rt) { memory: rt.memoryEnabled !== false, // Set while another dsh host holds the team (store.js): why this one cannot change it. readOnly: rt.readOnly() ?? null, @@ -517,7 +517,7 @@ diff --git a/src/host/channel.js b/src/host/channel.js function setPrefs(body) { if (body.theme !== undefined) { -@@ -97,28 +97,64 @@ +@@ -97,28 +97,64 @@ export function install(rt) { else if (body.motion === 'quiet' || body.motion === 'lively') state.prefs.motion = body.motion else throw invalid('Motion must be quiet, normal or lively') } @@ -586,7 +586,7 @@ diff --git a/src/host/channel.js b/src/host/channel.js case 'duplicate-bot': await rt.load() return ok(await rt.duplicateBot(body.id)) -@@ -141,7 +141,7 @@ +@@ -141,7 +141,7 @@ export function install(rt) { return ok([...rt.roomProgress.entries()].filter(([, entry]) => entry.roomId === body.roomId).map(([callId, entry]) => ({ callId, replies: entry.replies, speaking: entry.speaking ?? null }))) case 'create-bot': { await rt.bootstrap() @@ -595,7 +595,7 @@ diff --git a/src/host/channel.js b/src/host/channel.js return ok(bot) } case 'update-bot': -@@ -150,18 +150,23 @@ +@@ -150,18 +150,23 @@ export function install(rt) { case 'delete-bot': await rt.load() await rt.deleteBot(body.id) @@ -623,7 +623,7 @@ diff --git a/src/host/channel.js b/src/host/channel.js return ok(room) } case 'update-room': { -@@ -171,12 +171,70 @@ +@@ -171,12 +171,70 @@ export function install(rt) { notice: body.notice, mode: body.mode, admin: body.admin, @@ -696,7 +696,7 @@ diff --git a/src/host/channel.js b/src/host/channel.js } case 'folders': case 'folder-pick': -@@ -203,17 +203,19 @@ +@@ -203,17 +203,19 @@ export function install(rt) { case 'remove-agent': await rt.load() await rt.removeAgent(body.agentId) @@ -719,7 +719,7 @@ diff --git a/src/host/channel.js b/src/host/channel.js case 'stop-member-work': return ok(await rt.stopMemberWork(String(body.roomId ?? ''), String(body.botId ?? ''))) case 'dismiss-question': -@@ -231,6 +231,9 @@ +@@ -231,6 +231,9 @@ export function install(rt) { await rt.load() return ok(await rt.secretEndpoint(endpoint, body)) case 'connector-connect': @@ -729,7 +729,7 @@ diff --git a/src/host/channel.js b/src/host/channel.js case 'connector-disconnect': case 'connector-retry': await rt.load() -@@ -241,11 +241,22 @@ +@@ -241,11 +241,22 @@ export function install(rt) { case 'bot-capability': await rt.load() return ok(await rt.capabilityEndpoint(endpoint, body)) @@ -753,7 +753,7 @@ diff --git a/src/host/channel.js b/src/host/channel.js case 'memory-view': case 'memory-ask': case 'memory-forget': -@@ -258,8 +258,17 @@ +@@ -258,8 +258,17 @@ export function install(rt) { const sessionId = rt.botOf(body.sessionId) ? rt.chatOf(body.sessionId) : past && past.roomId === undefined ? rt.chatOf(past.botId) : body.sessionId const question = state.questions[sessionId] if (question === undefined || question.id !== body.questionId) return fail('This question is no longer open', 'bot/stale') @@ -771,7 +771,7 @@ diff --git a/src/host/channel.js b/src/host/channel.js // Claim the question before queueing so a second answer is refused. delete state.questions[sessionId] try { -@@ -278,9 +278,49 @@ +@@ -278,9 +278,49 @@ export function install(rt) { const text = String(body.text ?? '').trim() if (text === '' || typeof body.sessionId !== 'string') return fail('Nothing to send', 'bot/invalid') await rt.load() @@ -821,7 +821,7 @@ diff --git a/src/host/channel.js b/src/host/channel.js await rt.ensureCapabilities(sessionId) await ctx.sessionController.prompt({ requestId: randomUUID(), -@@ -319,32 +319,120 @@ +@@ -319,32 +319,120 @@ export function install(rt) { case 'diagnostics-clear': await rt.diagnostics.clear() return ok(true) @@ -947,7 +947,6 @@ diff --git a/src/host/channel.js b/src/host/channel.js + dispose?.() + } +} - diff --git a/src/host/delivery.js b/src/host/delivery.js --- a/src/host/delivery.js +++ b/src/host/delivery.js @@ -1185,7 +1184,7 @@ diff --git a/src/host/delivery.js b/src/host/delivery.js diff --git a/test/rounds.test.js b/test/rounds.test.js --- a/test/rounds.test.js +++ b/test/rounds.test.js -@@ -114,6 +114,8 @@ +@@ -114,6 +114,8 @@ const waitFor = async (check) => { return undefined } const turnFor = (sent, ...earlier) => waitFor(() => sent.find(entry => !earlier.includes(entry) && entry.message.source.role === 'room')) @@ -1194,7 +1193,7 @@ diff --git a/test/rounds.test.js b/test/rounds.test.js test('a new group Session waits for capabilities before its first member prompt', async (t) => { const { call, emit, userMessage, relay, answer, stop, sent, rt } = start() -@@ -180,6 +180,69 @@ +@@ -180,6 +180,69 @@ test('a message the composer queues mid-round joins the round and leaves the Ses assert.deepEqual(inboxOf(room.id).nextTurn, []) }) @@ -1264,7 +1263,7 @@ diff --git a/test/rounds.test.js b/test/rounds.test.js test('a plain message steered mid-round is answered by everyone, admin first, inside the round', async (t) => { const { call, emit, userMessage, compose, relay, inboxOf, answer, stop, sent } = start() t.after(stop) -@@ -253,32 +253,6 @@ +@@ -253,32 +253,6 @@ test('in admin mode a plain mid-round message goes to the admin', async (t) => { assert.deepEqual(result.replies.map(reply => reply.name ?? 'User'), ['Chief', 'Writer', 'User', 'Chief']) }) @@ -1297,7 +1296,7 @@ diff --git a/test/rounds.test.js b/test/rounds.test.js test('a message with an image or a chat reference waits for its own round, and so does what follows it', async (t) => { const { call, emit, userMessage, compose, relay, inboxOf, answer, stop, sent } = start() t.after(stop) -@@ -300,19 +300,26 @@ +@@ -300,19 +300,26 @@ test('a message with an image or a chat reference waits for its own round, and s assert.equal(inboxOf(room.id).nextStep.length, 1) }) @@ -1331,7 +1330,7 @@ diff --git a/test/rounds.test.js b/test/rounds.test.js }) test('a stopped round leaves a waiting message for the next round', async (t) => { -@@ -331,6 +331,377 @@ +@@ -331,6 +331,377 @@ test('a stopped round leaves a waiting message for the next round', async (t) => assert.equal(inboxOf(room.id).nextTurn.length, 1) }) @@ -1709,7 +1708,7 @@ diff --git a/test/rounds.test.js b/test/rounds.test.js test('a message sent between rounds still opens a full round', async (t) => { const { call, emit, userMessage, relay, answer, stop, sent } = start() t.after(stop) -@@ -347,3 +347,1325 @@ +@@ -347,3 +347,1325 @@ test('a message sent between rounds still opens a full round', async (t) => { const result = JSON.parse(await running) assert.deepEqual(result.replies.map(reply => reply.name ?? 'User'), ['Chief', 'Writer']) }) diff --git a/patches/ds-bot/0012-room-membership.patch b/patches/ds-bot/0012-room-membership.patch index 88820d006..7b164b83e 100644 --- a/patches/ds-bot/0012-room-membership.patch +++ b/patches/ds-bot/0012-room-membership.patch @@ -798,7 +798,7 @@ diff --git a/src/host/groups.js b/src/host/groups.js const ROOM_LOG_MAX = 60 const ROOM_FIRST_LOOK = 12 -@@ -31,6 +31,10 @@ +@@ -31,6 +31,10 @@ const ROOM_EXTRA_TURNS = 4 const ROOM_ADMIN_EXTRA_TURNS = 4 const ROOM_TURNS_EACH = 2 const ROOM_ADMIN_TURNS_EACH = 4 @@ -809,7 +809,7 @@ diff --git a/src/host/groups.js b/src/host/groups.js // Logs written before rooms were versioned get sequence numbers in place. const ensureRoomLog = (room) => { -@@ -41,9 +41,64 @@ +@@ -41,9 +41,64 @@ const ensureRoomLog = (room) => { room.seen ??= {} } @@ -875,7 +875,7 @@ diff --git a/src/host/groups.js b/src/host/groups.js } // A member's own lines are already in its Session, so only the others' are news. -@@ -58,6 +58,9 @@ +@@ -58,6 +58,9 @@ const lineText = (line) => { if (line.kind === 'resumed') return `[${line.who}'s work here was cut off when DeepSeek Harness restarted; it is picking it back up and posts when done]` if (line.kind === 'cut') return `[${line.who}'s work here was cut off by a restart again, so it stopped; ask it to go on if it still matters]` if (line.kind === 'stopped') return `[The user stopped ${line.who}'s work here]` @@ -885,7 +885,7 @@ diff --git a/src/host/groups.js b/src/host/groups.js return `${line.who}: ${line.text}` } const transcript = lines => lines.map(lineText).join('\n') -@@ -86,7 +86,17 @@ +@@ -86,7 +86,17 @@ export const referenceNote = references => (references?.length : '') const ROOM_REPLY = 'Reply with your message for the group, no name prefix: in a discussion, one or two short messages. When you are asked to do something, do it now with your tools. The group does not wait for you, and your final reply is posted here when you finish, so give the result, never a promise to do it later. Reply [PASS] to stay silent when you have nothing new to add or the message is for someone else.' @@ -903,7 +903,7 @@ diff --git a/src/host/groups.js b/src/host/groups.js const slotLabel = (slot) => { if (slot.status === 'speaking') return `${slot.name} (you, now)` -@@ -100,162 +100,242 @@ +@@ -100,162 +100,242 @@ const slotLabel = (slot) => { return slot.name } @@ -945,6 +945,12 @@ diff --git a/src/host/groups.js b/src/host/groups.js // A member's group Session, created on its first turn there. A new one starts with // a fresh look at the log and a brief of the member's own chat; the brief is the // first message, so it stays fixed and the prefix stays cacheable. +- async function groupSessionFor(room, member) { +- room.sessions ??= {} +- const known = room.sessions[member.id] +- if (known !== undefined) { +- await rt.ensureCapabilities(known) +- return { id: known, brief: undefined } + // The brief is the member's owner's private chat with the Bot. In a room where + // another owner sits it stays out unless that owner shared it with this room (#53). + const briefAllowed = (room, member) => { @@ -956,19 +962,7 @@ diff --git a/src/host/groups.js b/src/host/groups.js + // A granted remote Bot speaks for another home; its handle stands for that owner. + else if (p.remote) owners.add(p.handle) + else owners.add(rt.botOf(p.id)?.ownerId) -+ } -+ owners.delete(undefined) -+ if (owners.size <= 1) return true -+ return participants.some(p => p.kind === 'bot' && p.id === member.id && p.briefShared === true) -+ } -+ - async function groupSessionFor(room, member) { -- room.sessions ??= {} -- const known = room.sessions[member.id] -- if (known !== undefined) { -- await rt.ensureCapabilities(known) -- return { id: known, brief: undefined } -- } + } - const id = await rt.createSession(`${member.name} · ${room.name}`, { dir: rt.workDirOf({ botId: member.id, roomId: room.id }) }) - room.sessions[member.id] = id - rt.groupOwners.set(id, { botId: member.id, roomId: room.id }) @@ -977,6 +971,12 @@ diff --git a/src/host/groups.js b/src/host/groups.js - if (ref !== undefined) { - await rt.selectSessionModel(id, splitRef(ref)) - .catch(error => rt.warn('ds-bot: model for %s in %s not applied: %s', member.name, room.name, error)) ++ owners.delete(undefined) ++ if (owners.size <= 1) return true ++ return participants.some(p => p.kind === 'bot' && p.id === member.id && p.briefShared === true) ++ } ++ ++ async function groupSessionFor(room, member) { + const session = await participant.open({ room, member }) + return { id: session.id, brief: session.created && briefAllowed(room, member) ? await rt.ownChatBrief(member) : undefined } + } @@ -1252,7 +1252,7 @@ diff --git a/src/host/groups.js b/src/host/groups.js if (by === undefined) delete called.by else called.by = by round.slots.splice(next, 0, called) -@@ -285,34 +285,138 @@ +@@ -285,34 +285,138 @@ export function install(rt) { // Session's inbox, so it does not open another round when this one ends. Returns // the number of turns added, which the round's limit allows for. function drainRoomInbox(room, round, turns, index, agent) { @@ -1399,7 +1399,7 @@ diff --git a/src/host/groups.js b/src/host/groups.js if (index >= round.slots.length) break const slot = round.slots[index] const member = rt.botOf(slot.id) -@@ -321,6 +321,20 @@ +@@ -321,6 +321,20 @@ export function install(rt) { slot.status = 'left' continue } @@ -1420,7 +1420,7 @@ diff --git a/src/host/groups.js b/src/host/groups.js slot.status = 'speaking' round.speaking = member.id round.turns += 1 -@@ -339,7 +339,13 @@ +@@ -339,7 +339,13 @@ export function install(rt) { const prompt = roomPrompt(room, member, round) round.shown?.push(member.id) const text = session.brief === undefined ? prompt : `${session.brief}\n\n${prompt}` @@ -1435,7 +1435,7 @@ diff --git a/src/host/groups.js b/src/host/groups.js round.speaking = null if (reply?.failure !== undefined) { // Not a pass: the user sees why the member is silent, and the round goes on. -@@ -366,9 +366,18 @@ +@@ -366,9 +366,18 @@ export function install(rt) { if (reply?.missed) round.replies.push({ kind: 'missed', botId: member.id, name: member.name, color: member.color, time: Date.now() }) continue } @@ -1456,7 +1456,7 @@ diff --git a/src/host/groups.js b/src/host/groups.js void rt.save().catch(() => {}) // Only the admin's "@all" calls everyone, so a member cannot set off a storm. const leads = rt.isAdminOf(room, member.id) -@@ -377,7 +377,11 @@ +@@ -377,7 +377,11 @@ export function install(rt) { const { all, found: named } = mentionsIn(reply, everyone) const found = named.filter(other => other.id !== member.id) const callees = leads && all ? [...found, ...others.filter(other => !found.includes(other))] : found @@ -1469,7 +1469,7 @@ diff --git a/src/host/groups.js b/src/host/groups.js } } -@@ -397,38 +397,45 @@ +@@ -397,38 +397,45 @@ export function install(rt) { rt.roomInbox.delete(room.id) const posts = queued.length > 0 ? queued : [{ text: '' }] const startSeq = room.seq + 1 @@ -1535,7 +1535,7 @@ diff --git a/src/host/groups.js b/src/host/groups.js speaking: null, turns: 0, replies: [], -@@ -436,6 +436,7 @@ +@@ -436,6 +436,7 @@ export function install(rt) { failed: [], started: [], } @@ -1543,7 +1543,7 @@ diff --git a/src/host/groups.js b/src/host/groups.js for (const post of posts.filter(item => item.failed)) { const bot = rt.botOf(post.botId) const { code, status, message } = post.failed -@@ -450,17 +450,25 @@ +@@ -450,17 +450,25 @@ export function install(rt) { appendRoom(room, { who: name, botId: post.botId, kind, text: '' }) round.replies.push({ kind, botId: post.botId, name, color: bot?.color, time: Date.now() }) } @@ -1570,7 +1570,7 @@ diff --git a/src/host/groups.js b/src/host/groups.js } await rt.save() for (const post of posts) { -@@ -471,6 +471,41 @@ +@@ -471,6 +471,41 @@ export function install(rt) { }, })) @@ -1612,7 +1612,7 @@ diff --git a/src/host/groups.js b/src/host/groups.js // Private context enters a group only through this tool, and the group sees each use. ctx.tools.register(defineTool({ name: 'read_own_chat', -@@ -539,7 +539,9 @@ +@@ -539,7 +539,9 @@ export function install(rt) { const { room, error } = rt.pickRoom(args.group, self) if (error) return error ensureRoomLog(room) @@ -1623,7 +1623,7 @@ diff --git a/src/host/groups.js b/src/host/groups.js return lines.length === 0 ? `"${room.name}" has no messages yet.` : `Latest in "${room.name}" (oldest first):\n${transcript(lines)}` }, })) -@@ -548,46 +548,207 @@ +@@ -548,46 +548,207 @@ export function install(rt) { // the admin answers alone in admin mode, or first of everyone. What a member posts // after its round moved on is answered by the member that asked for it; work the // user asked for is left for the user to read. @@ -1836,7 +1836,7 @@ diff --git a/src/host/groups.js b/src/host/groups.js .catch(error => rt.warn('ds-bot: late group reply from %s failed: %s', member.name, error)) } rt.lateRoomReply = lateRoomReply -@@ -599,9 +599,8 @@ +@@ -599,9 +599,8 @@ export function install(rt) { const room = rt.roomOf(roomId) const member = rt.botOf(botId) const sessionId = room?.sessions?.[botId] @@ -1848,7 +1848,7 @@ diff --git a/src/host/groups.js b/src/host/groups.js await postToRoom(room, member, '', { quiet: true, stopped: true }) return true } -@@ -615,37 +615,42 @@ +@@ -615,37 +615,42 @@ export function install(rt) { const member = rt.botOf(owner?.botId) if (room === undefined || member === undefined || !room.members.includes(member.id)) return const { code, status, message } = failure @@ -1907,7 +1907,7 @@ diff --git a/src/host/groups.js b/src/host/groups.js .catch(error => rt.warn('ds-bot: group report to %s failed: %s', post.name, error)) } -@@ -665,7 +665,7 @@ +@@ -665,7 +665,7 @@ export function install(rt) { // message from the user, a scheduled task, or a Bot's post after the last model or // tool step starts a relay. const opens = message => message.role === 'user' @@ -1916,7 +1916,6 @@ diff --git a/src/host/groups.js b/src/host/groups.js const latest = [...options.messages].reverse() .find(message => message.role === 'assistant' || message.role === 'tool' || opens(message)) if (latest?.role !== 'user') { - diff --git a/test/at-menu.test.js b/test/at-menu.test.js --- a/test/at-menu.test.js +++ b/test/at-menu.test.js diff --git a/patches/ds-bot/files/src/host/delivery-board.js b/patches/ds-bot/files/src/host/delivery-board.js index e3b47a046..a3ae1791a 100644 --- a/patches/ds-bot/files/src/host/delivery-board.js +++ b/patches/ds-bot/files/src/host/delivery-board.js @@ -11,7 +11,7 @@ // `{ message, endpoint, status, kind, scope, at }` so the view can say what failed, where, and what // to do about it. "No GitHub connection yet" is reserved for a host that has never tried. import { repoSlug } from './update.js' -import { NO_REPO_MESSAGE, resolveRepo } from './delivery-repo.js' +import { NO_REPO_MESSAGE, instanceRepoSetting, resolveRepo } from './delivery-repo.js' export const DELIVERY_COLUMNS = ['raw', 'ready', 'working', 'review', 'done-today'] @@ -761,7 +761,7 @@ export function install(rt) { try { return rt.ctx?.workspaceRegistry?.get?.(rt.state?.workspaceId)?.path ?? null } catch { return null } } const setting = () => { - try { return rt.config?.deliveryRepo ?? null } catch { return null } + try { return instanceRepoSetting(rt.config) } catch { return null } } // Level 3 is the package's own repository, not the instance's choice. The shipped package carries // it; a host whose package.json has none (or a test) hands in '' and the board refuses rather than diff --git a/patches/ds-bot/files/src/host/delivery-repo.js b/patches/ds-bot/files/src/host/delivery-repo.js index 62944dab6..16ef630e2 100644 --- a/patches/ds-bot/files/src/host/delivery-repo.js +++ b/patches/ds-bot/files/src/host/delivery-repo.js @@ -5,12 +5,14 @@ // were. One repository is resolved here, for the host and for `scripts/delivery-probe.mjs` alike, // in three steps, first answer wins: // -// 1. the instance setting, `delivery.repo` on the plugin entry (`config.deliveryRepo`); +// 1. the instance setting, `delivery.repo` on the plugin entry — the host hands the entry through +// verbatim, so all three spellings people write are read (`instanceRepoSetting`); // 2. the workspace's own git remote — `/.git/config`, `[remote "origin"] url`; // 3. the package's `repository.url`, which is a default and is labelled as one. // -// Nothing is spawned: the host may run where `git` is not on PATH. A `.git` that is a file (a -// worktree or a submodule) is followed through its `gitdir:` pointer instead. A name is normalised +// Nothing is spawned: the host may run where `git` is not on PATH. A `.git` that is a file is +// followed through its `gitdir:` pointer: a submodule keeps `config` there, and a linked worktree +// keeps only a `commondir` pointer to the shared directory that does (#1258). A name is normalised // to `owner/name`; a GitHub URL or the scp form `git@github.com:owner/name.git` is accepted where a // name is, because that is what people paste. import { readFileSync, statSync } from 'node:fs' @@ -19,7 +21,10 @@ import { join, resolve } from 'node:path' // The wording acceptance criterion 4 asks for, in one place so the view, the host and the CLI agree. export const NO_REPO_MESSAGE = 'no repository — set delivery.repo' -const OWNER_NAME = /^[\w.-]+\/[\w.-]+$/ +// A segment is a plain repository name part: word characters, dots and dashes, but never a leading +// dot. `.` and `..` are path steps, not names, and a board that accepted them would build +// `/repos/../../issues` and normalise it to the API root (#1258). +const OWNER_NAME = /^(?!\.)[\w.-]+\/(?!\.)[\w.-]+$/ const SCP_GITHUB = /^(?:[\w.-]+@)?github\.com:([\w.-]+\/[\w.-]+?)(?:\.git)?\/?$/i const cleanName = text => { @@ -61,18 +66,51 @@ export function originUrlIn(text) { return null } -// Where git keeps the config for this folder: `.git/config`, or the `gitdir:` a `.git` file names. -// Null when the folder is not a git checkout at all. +// Where git keeps the config for this folder: `.git/config`, or whatever a `.git` file's `gitdir:` +// pointer leads to. A submodule keeps `config` in that directory; a linked worktree keeps only a +// `commondir` file naming the shared directory that has it (#1258). Null when the folder is not a +// git checkout at all. export function gitConfigPath(folder) { if (typeof folder !== 'string' || folder === '') return null const dot = join(folder, '.git') try { if (statSync(dot).isDirectory()) return join(dot, 'config') const pointer = /^gitdir:\s*(.+)$/m.exec(readFileSync(dot, 'utf8')) - return pointer ? join(resolve(folder, pointer[1].trim()), 'config') : null + if (!pointer) return null + const gitdir = resolve(folder, pointer[1].trim()) + const own = join(gitdir, 'config') + if (isFile(own)) return own + const common = commonDirOf(gitdir) + return common === null ? null : join(common, 'config') } catch { return null } } +const isFile = path => { + try { return statSync(path).isFile() } catch { return false } +} + +// A linked worktree's gitdir holds no config: `commondir` names the common directory, relative to +// the gitdir, where the shared `config` lives. +const commonDirOf = gitdir => { + try { + const pointer = readFileSync(join(gitdir, 'commondir'), 'utf8').trim() + return pointer === '' ? null : resolve(gitdir, pointer) + } catch { return null } +} + +// The instance setting as the profile actually wrote it. The host passes the plugin entry through +// verbatim and expands no dotted key, so one setting reaches us as a flat `deliveryRepo`, a literal +// `'delivery.repo'`, or a nested `delivery.repo` — the hint, the page and the docs all say the last +// spelling, and reading only the first would ignore what a user just wrote (#1258). First defined +// answer wins; validation stays in `parseRepoName`. +export function instanceRepoSetting(config) { + if (config === null || typeof config !== 'object') return null + for (const value of [config.delivery?.repo, config['delivery.repo'], config.deliveryRepo]) { + if (value !== undefined && value !== null && value !== '') return value + } + return null +} + // The `owner/name` the folder's `origin` names, read from disk without running git. export function gitOriginRepo(folder) { const path = gitConfigPath(folder) diff --git a/patches/ds-bot/files/test/delivery-board.test.js b/patches/ds-bot/files/test/delivery-board.test.js index 3a29d288c..bd0daa8f6 100644 --- a/patches/ds-bot/files/test/delivery-board.test.js +++ b/patches/ds-bot/files/test/delivery-board.test.js @@ -852,6 +852,20 @@ test('the instance setting names the repository for the board and the probe, and assert.ok(probe.rows.every(row => row.ok)) }) +test('a dotted or nested delivery.repo names the repository too, as a user writes it (#1258)', async () => { + // The hint says `delivery.repo`, so that is what a profile carries — as a literal dotted key or + // nested; a literal key wins nowhere, it is simply read. The flat `deliveryRepo` is the third. + for (const config of [{ 'delivery.repo': 'acme/widget' }, { delivery: { repo: 'acme/widget' } }]) { + const github = fakeGitHub({ slug: 'acme/widget' }) + const installed = hostWith(github, { config }) + const settled = await installed.deliveryBoard({ wait: true }) + assert.equal(settled.repo, 'acme/widget', JSON.stringify(config)) + assert.equal(settled.repoSource, 'config', JSON.stringify(config)) + assert.equal(settled.state, 'fresh') + for (const call of github.calls) assert.match(call.url, /^https:\/\/api\.github\.com\/repos\/acme\/widget(\/|$|\?)/, call.url) + } +}) + test('the workspace git remote names the repository when there is no setting (#1250)', async () => { const dir = mkdtempSync(join(tmpdir(), 'delivery-install-')) try { diff --git a/patches/ds-bot/files/test/delivery-repo.test.js b/patches/ds-bot/files/test/delivery-repo.test.js index d25659333..5cb2b59d0 100644 --- a/patches/ds-bot/files/test/delivery-repo.test.js +++ b/patches/ds-bot/files/test/delivery-repo.test.js @@ -7,7 +7,7 @@ import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'nod import { tmpdir } from 'node:os' import { join } from 'node:path' import { test } from 'node:test' -import { gitConfigPath, gitOriginRepo, NO_REPO_MESSAGE, originUrlIn, parseRepoName, resolveRepo } from '../src/host/delivery-repo.js' +import { gitConfigPath, gitOriginRepo, instanceRepoSetting, NO_REPO_MESSAGE, originUrlIn, parseRepoName, resolveRepo } from '../src/host/delivery-repo.js' const scratch = () => mkdtempSync(join(tmpdir(), 'delivery-repo-')) const cleanup = dir => rmSync(dir, { recursive: true, force: true }) @@ -56,18 +56,43 @@ test('an origin written as a URL, an scp path or a name with no .git reads the s } }) -test('a .git file is followed through its gitdir, and git is never spawned (#1250)', () => { - // A worktree (or a submodule) keeps its config elsewhere and `.git` is a pointer file. +test('a submodule keeps its config in the gitdir the `.git` file names (#1250)', () => { + // A submodule's `.git` is a pointer file and its own gitdir really does hold `config`. const root = scratch() try { - const store = join(root, 'main.git', 'worktrees', 'wt') + const store = join(root, 'super', '.git', 'modules', 'sub') mkdirSync(store, { recursive: true }) - writeFileSync(join(store, 'config'), '[remote "origin"]\n\turl = git@github.com:acme/worktree.git\n') + writeFileSync(join(store, 'config'), '[remote "origin"]\n\turl = git@github.com:acme/submodule.git\n') + const dir = join(root, 'super', 'sub') + mkdirSync(dir, { recursive: true }) + writeFileSync(join(dir, '.git'), `gitdir: ${join('..', '.git', 'modules', 'sub')}\n`) + assert.equal(gitConfigPath(dir), join(store, 'config')) + assert.deepEqual(resolveRepo({ workspace: dir }), { repo: 'acme/submodule', source: 'remote' }) + } finally { cleanup(root) } +}) + +test('a linked worktree follows its `commondir` to the config the shared directory holds (#1258)', () => { + // `git worktree add` writes `.git` -> `
/.git/worktrees/`, and that directory holds no + // config of its own: only `commondir`, naming the shared directory where git keeps it. Reading + // `config` beside the pointer finds nothing, so a worktree workspace fell to the package default. + const root = scratch() + try { + const shared = join(root, 'main', '.git') + mkdirSync(join(shared, 'worktrees', 'wt'), { recursive: true }) + writeFileSync(join(shared, 'config'), '[remote "origin"]\n\turl = git@github.com:acme/worktree.git\n') + writeFileSync(join(shared, 'worktrees', 'wt', 'commondir'), '../..\n') const dir = join(root, 'checkout') mkdirSync(dir) - writeFileSync(join(dir, '.git'), `gitdir: ${join('..', 'main.git', 'worktrees', 'wt')}\n`) - assert.equal(gitConfigPath(dir), join(store, 'config')) + writeFileSync(join(dir, '.git'), `gitdir: ${join('..', 'main', '.git', 'worktrees', 'wt')}\n`) + assert.equal(gitConfigPath(dir), join(shared, 'config')) assert.deepEqual(resolveRepo({ workspace: dir }), { repo: 'acme/worktree', source: 'remote' }) + // A pointer with neither a config nor a commondir answers nothing rather than a wrong path. + const bare = join(root, 'bare') + mkdirSync(join(bare, 'gitdir'), { recursive: true }) + mkdirSync(join(root, 'elsewhere')) + writeFileSync(join(root, 'elsewhere', '.git'), 'gitdir: ../bare/gitdir\n') + assert.equal(gitConfigPath(join(root, 'elsewhere')), null) + assert.deepEqual(resolveRepo({ workspace: join(root, 'elsewhere') }), { repo: null, source: null }) } finally { cleanup(root) } }) @@ -108,6 +133,39 @@ test('a value that is not a GitHub repository is refused, not mined for its last assert.equal(originUrlIn('[remote "origin"]\n\tfetch = +refs/heads/*:refs/remotes/origin/*\n'), null) }) +test('a path step is not a repository name, so `.` and `..` segments are refused (#1258)', () => { + // `OWNER_NAME` allowed a dot anywhere, so `../..` answered a "repository": the board then built + // `/repos/../../issues`, which normalises to the API root. Same host, so no token left, but every + // request it made was meaningless and the "this is a repository name" invariant was gone. + for (const value of ['../..', './widget', '.a/b', 'acme/.b', 'acme/..']) { + assert.equal(parseRepoName(value), null, value) + } + // A dot inside a segment is a real repository name and stays allowed. + for (const value of ['acme/widget.js', 'a.b/c-d', 'a1-2_3/b.4']) { + assert.equal(parseRepoName(value), value, value) + } +}) + +test('the instance setting is read in all three spellings a profile can write it (#1258)', () => { + // The host passes the plugin entry through verbatim and expands no dotted key, so the hint's + // `delivery.repo` reaches the board as one of three shapes. Reading only the flat one ignored + // what a user had just written and fell through to the labelled package default. + assert.equal(instanceRepoSetting({ delivery: { repo: 'acme/widget' } }), 'acme/widget') + assert.equal(instanceRepoSetting({ 'delivery.repo': 'acme/widget' }), 'acme/widget') + assert.equal(instanceRepoSetting({ deliveryRepo: 'acme/widget' }), 'acme/widget') + // The documented spelling wins over the runtime shape when a profile happens to carry both. + assert.equal(instanceRepoSetting({ delivery: { repo: 'acme/nested' }, deliveryRepo: 'acme/flat' }), 'acme/nested') + // Nothing written, or an empty value, is "no setting", not the empty string as a repository. + assert.equal(instanceRepoSetting({}), null) + assert.equal(instanceRepoSetting({ delivery: {} }), null) + assert.equal(instanceRepoSetting({ deliveryRepo: '' }), null) + assert.equal(instanceRepoSetting(null), null) + assert.equal(instanceRepoSetting(undefined), null) + // A nested setting is a real setting end to end, not only in the helper. + assert.deepEqual(resolveRepo({ setting: instanceRepoSetting({ delivery: { repo: 'acme/widget' } }), packageSlug: 'x/y' }), + { repo: 'acme/widget', source: 'config' }) +}) + test('the probe is a shell over this resolver, and refuses before it asks (#1250)', () => { const source = readFileSync(new URL('../scripts/delivery-probe.mjs', import.meta.url), 'utf8') // One resolver, imported — a second copy in the CLI is how the two drift.