diff --git a/docs/designs/data-directory-layout.md b/docs/designs/data-directory-layout.md index b21050c9d..b48a8378a 100644 --- a/docs/designs/data-directory-layout.md +++ b/docs/designs/data-directory-layout.md @@ -453,7 +453,7 @@ Upgrading from the per-checkout layout: migrated still says `kind: self`, and its queue would publish to the new repository. It is set aside instead, as a mode switch does (below), to `pending-learnings.self` beside the partition queue, with a warning naming - it; `contribute` and `import --from-mr` take the same step before they queue. + it. A queue with no `config.yaml` beside it is one an older self install left after its config had moved to the partition; once the project serves another install no plan covers it, so every migrating command sets it aside the same @@ -518,7 +518,31 @@ Upgrading from the per-checkout layout: linked worktree works with no pull first. One with uncommitted changes stays: the side-branch step fails with an error naming the path and the next step (commit or move the changes, or delete the path by hand), and the queue keeps - the learnings until then. The error is also printed as a warning, because + the learnings until then. What `import --from-mr` in 0.25.0 to 0.26.0-beta.3 left there is not + such a change (#823 item 7): it wrote `learnings/-.md` with + `source_mr` in the frontmatter and never committed it. `publishQueuedLearnings` + first finds the branch's one registered checkout (`git worktree list` of the + owning repo, so the shared one or the old `.teamai/<dirname>`, never another + repository's), and queues each untracked file of exactly that shape, directly + under `learnings/`, into the active namespace with `contribute`'s name, then + deletes the original. A file the branch (any namespace) or the queue + already has, by `source_mr` or by content, is deleted without queueing. The + branch is the tree of `origin/teamai-learnings`, fetched first, never the + checkout's tracked files: the old checkout is never synced again, so it may + miss a teammate's later import of the same MR, or still track one origin has + since deleted (#823 item 21). When that fetch fails, every such file stays where it is + until a run can fetch, so a stale ref never queues a duplicate; when + `git ls-remote` shows origin has no such branch (an offline first publish + never pushed it), origin adds nothing and the files are queued. A file of + that shape that cannot be read is skipped, never holding back the rest. The + queue is read under the queue lock, and only while the data home's config + still names the install the command loaded (`readPendingForInstall`, the + check `listPendingForInstall` makes): after `init` switched the project to + another team repository the queue holds that install's learnings, so while it + does, or while the lock stays busy, every such file stays where it is. It runs only under the sync lock, before the queue is listed, + and never on a dry run, which publishes nothing from the queue either and + `pull` reports as `Would publish N queued learning(s)` (#823 item 20); any + other file in the checkout is left alone. The error is also printed as a warning, because several callers treat a side-branch failure as non-fatal and log it at debug only; a silent (hook) run prints nothing. `refresh` does not swallow it: `recall maintenance` and `recall promote` stop with exit code 1 and write @@ -535,7 +559,10 @@ Upgrading from the per-checkout layout: reader (`members`, `projects members`, `digest`, `pull`, `stats` and `viz`, through `readableReportsWorktree`) uses the local copy and never ensures it, after the same ownership probe as `indexableVotesDir` (below): another - repository's copy is refused with `ForeignCheckoutError`. The + repository's copy is refused with `ForeignCheckoutError`. A refresh that + failed is tried once more, under the lock, and a second failure throws its + cause: a reader never creates the checkout without the lock, which a writer + that took it meanwhile may be creating (#823 item 15). The checkout there may be another repository's, or not created yet, so `recall maintenance` and `recall promote` stop with exit code 1 and write nothing when either the reports lock (they rank by its votes) or the @@ -595,6 +622,18 @@ exists but cannot be read names no owner: the loaders return null for it as for a fresh install, so `init` checks for the file, and with learnings queued moves them to `pending-learnings.unknown` the same way, the warning naming that config; queued or not, it drops the search indexes, as on an owner change. +When `init` is about to clone another owner's team repository where the old +install's clone was, or reuses a clone of it that an earlier `init` left there, +it settles the old install right then, before the clone or the refresh, +the same way (queue set aside, indexes dropped), and moves the old +`config.yaml` to the first free `config.yaml.previous[.<n>]` instead of saving +(#823 item 17). The save at the end still compares with the old install +loaded before the clone, finds nothing left to set aside, and writes the new +config. An `init` that stops in between (an unknown `--role`, a busy queue lock, a +prompt left) leaves no config naming the old team beside the new clone, so every +command asks for `teamai init`; nothing is deleted. The rerun finds no +`config.yaml`, so it reads the last `config.yaml.previous[.<n>]` for the +settings a re-init carries forward (`inheritUserScope`, agents, tool roots). A self install whose business repository moved to another URL (renamed or transferred) is another owner too: `init` sets its queue aside, and the warning names the directory to diff --git a/docs/usage-guide.md b/docs/usage-guide.md index c029fe51c..ca3e87d44 100644 --- a/docs/usage-guide.md +++ b/docs/usage-guide.md @@ -411,7 +411,13 @@ a side-branch checkout removes the old one. An old checkout with uncommitted changes is kept, and the command names it: nothing is published to or recalled from that branch until you commit, move or delete those changes, and `recall maintenance` and `recall promote` stop. Queued learnings stay queued -and recallable. Maintenance and promote also stop, naming the cause, when the +and recallable. A learning an older `import --from-mr` (0.25.0 to 0.26.0-beta.3) wrote into a +learnings checkout and never committed does not count: the next `pull` or +`contribute` (or an `import --from-mr` that queues a learning) queues and publishes it (in the project's +namespace, named as `contribute` names it) and says where it was, so the old +checkout can go. If the branch or the queue already has it, by its `source_mr` +or its content, it is deleted instead, and the message names the learning that +has it. Maintenance and promote also stop, naming the cause, when the checkout cannot be created, such as when `teamai-learnings` is checked out somewhere else. A git-mode install of the same project keeps its checkouts at the same paths. @@ -425,7 +431,12 @@ for the old repository (the next `recall` rebuilds them). Re-running `init` against another team repository of the same kind does the same, to `pending-learnings.<kind>-<repo>` (for example `pending-learnings.git-github.com-org-team-a`); the same repository written -another way (with or without `.git`, SSH or HTTPS) keeps the queue. When the old +another way (with or without `.git`, SSH or HTTPS) keeps the queue. Before it +clones the other team repository, or reuses a clone of it an earlier `init` left, `init` moves the old `config.yaml` to +`config.yaml.previous` beside it and says so: if `init` stops before it saves +the new config, every command asks for `teamai init` instead of running the old +team's config against the new clone. Run `init` again: it carries that config's +settings (agents, tool roots) from `config.yaml.previous`. When the old install's `config.yaml` exists but cannot be read, nothing says whose the queue is: `init` moves it to `pending-learnings.unknown`, names that file and deletes the search indexes. A checkout that had not been upgraded yet keeps its old queue the same way: the next command there @@ -1263,6 +1274,8 @@ After `--update-quality`, review the generated `.draft.md` files and rename them While another teamai command holds the learnings or reports checkout's lock, `recall maintenance` and `recall promote` exit 1 without writing anything (`The learnings checkout is locked: …`). Run them again when that command finishes. +Maintenance and promote publish only the learnings they changed. A file in the learnings checkout that nobody committed stays out of their commit. When the publish cannot run or push (`Maintenance changes stay local for now: …`), the next `teamai pull` or `contribute` publishes the change, even with no learning queued. A learning edited by hand after that run is not published as part of it: the edit stays uncommitted, and a warning names the file. A file someone staged in the checkout stays staged across the publish; when origin changed it too and it can no longer be staged as it was, it keeps its content as an unstaged change, and a warning names it. + ### Promoting Learnings When a learning reaches maturity, promote it to formal team knowledge (a skill, rule, or doc). Promotion criteria: confidence ≥ 0.90, ≥ 5 upvotes, ≥ 2 distinct contributors, age ≥ 14 days. diff --git a/docs/usage-guide.zh-CN.md b/docs/usage-guide.zh-CN.md index f76cbb2f8..5cd5e0418 100644 --- a/docs/usage-guide.zh-CN.md +++ b/docs/usage-guide.zh-CN.md @@ -375,7 +375,10 @@ git 同一分支只能在一个 worktree 中检出，所以仓库的所有检出 的命令会移除旧检出。旧检出中若有未提交的改动则保留，命令会指出其路径：在你提交、 移走或删除这些改动之前，不会向该分支发布内容，也不会从该分支召回内容，`recall maintenance` 与 `recall promote` 会停止。已排队的 learning -仍留在队列中，仍可被召回。检出无法创建时（例如 `teamai-learnings` 已在别处检出），maintenance 与 promote +仍留在队列中，仍可被召回。旧版 `import --from-mr`（0.25.0 至 0.26.0-beta.3）写进 learnings 检出却从未提交的 learning +不算在内：下一次 `pull` 或 `contribute`（或排队了 learning 的 `import --from-mr`）会把它排队并发布（放入项目的命名空间，按 `contribute` +的方式命名），并指出它原来的位置，旧检出因此可以移除。若分支或队列中已有同一条（按 `source_mr` 或内容判断）， +则删除它，提示中会给出已有的那条 learning。检出无法创建时（例如 `teamai-learnings` 已在别处检出），maintenance 与 promote 同样会停止，并说明原因。 同一项目的 git 模式安装把检出放在相同的路径。切换模式后，teamai 会拒绝使用属于另一个 仓库的检出，并打印清除它的 `git worktree remove` 命令：不会向它发布、不会为它建索引（包括其中的投票）、 @@ -383,7 +386,9 @@ git 同一分支只能在一个 worktree 中检出，所以仓库的所有检出 learning 会被移到同一数据目录下的 `pending-learnings.<旧类型>`，新安装不会发布它们；`init` 会说明数量和位置，并删除为旧仓库构建的搜索索引（下一次 `recall` 会重建）。以同一类型对另一个团队仓库重新运行 `init` 也同样处理，队列移到 `pending-learnings.<类型>-<仓库>`（例如 `pending-learnings.git-github.com-org-team-a`）； -同一仓库换一种写法（带或不带 `.git`、SSH 或 HTTPS）不会移动队列。若旧安装的 `config.yaml` 存在但无法读取， +同一仓库换一种写法（带或不带 `.git`、SSH 或 HTTPS）不会移动队列。克隆另一个团队仓库之前（或复用之前某次 `init` 留下的该仓库克隆之前）， +`init` 会把旧的 `config.yaml` 移到旁边的 `config.yaml.previous` 并给出提示：若 `init` 在保存新配置前停止， +所有命令都会要求先运行 `teamai init`，而不会用旧团队的配置操作新的克隆。重新运行 `init` 即可：它会从 `config.yaml.previous` 沿用该配置的设置（agent、工具根目录）。若旧安装的 `config.yaml` 存在但无法读取， 就无从得知队列属于谁：`init` 会把它移到 `pending-learnings.unknown`，指出该文件，并删除搜索索引。尚未升级的检出中的旧队列也同样处理： 在该检出运行的下一个命令会把它移到 `pending-learnings.self` 并给出路径。反过来，当某个检出的 `init --self` 把 git 模式项目切换为单仓库模式，而另一个仍保留旧安装的检出从 main 取得了知识时，在那里运行的下一个 @@ -1172,6 +1177,8 @@ teamai recall maintenance --update-quality 另一个 teamai 命令持有 learnings 或 reports checkout 的锁时，`recall maintenance` 与 `recall promote` 会以退出码 1 停止，不写入任何内容（`The learnings checkout is locked: …`）。待该命令结束后再运行。 +maintenance 与 promote 只发布它们改动过的 learning。learnings 检出中无人提交的文件不会进入它们的提交。发布无法进行或推送失败时（`Maintenance changes stay local for now: …`），下一次 `teamai pull` 或 `contribute` 会发布这些改动，即使队列中没有 learning。在那次运行之后被手动编辑过的 learning 不会作为它的一部分发布：该编辑保持未提交，警告会指出文件名。检出中有人暂存（staged）的文件在发布后仍保持暂存；若 origin 也改动了它、无法按原样重新暂存，它的内容会保留为未暂存的改动，警告会指出该文件。 + ### 晋升 Learnings 当 learning 达到成熟标准时，可将其晋升为正式团队知识（skill、rule 或 doc）。晋升判据：置信度 ≥ 0.90、≥ 5 次 upvote、≥ 2 个不同贡献者、存在时长 ≥ 14 天。 diff --git a/src/__tests__/e2e/learnings-remnants-823.test.ts b/src/__tests__/e2e/learnings-remnants-823.test.ts new file mode 100644 index 000000000..26a3cb3a0 --- /dev/null +++ b/src/__tests__/e2e/learnings-remnants-823.test.ts @@ -0,0 +1,275 @@ +/** + * E2E (#823 item 7, and the maintenance sweep beside it): git mode, user scope. + * + * `import --from-mr` (0.25.0 to 0.26.0-beta.3) wrote its learning into the learnings checkout and + * never committed it. Nothing published it, and `recall maintenance` staged + * all of `learnings/`, so the first maintenance commit swept it, or any other + * file nobody meant to share, into a commit about something else. + */ +import { afterEach, beforeAll, describe, expect, it } from 'vitest'; +import { execFileSync, spawn } from 'node:child_process'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const ROOT = path.resolve(__dirname, '..', '..', '..'); +const CLI = path.join(ROOT, 'dist', 'index.js'); + +const GIT_ENV = { + GIT_AUTHOR_NAME: 'TeamAI CI', + GIT_AUTHOR_EMAIL: 'ci@teamai.test', + GIT_COMMITTER_NAME: 'TeamAI CI', + GIT_COMMITTER_EMAIL: 'ci@teamai.test', +}; + +interface RunResult { + code: number | null; + output: string; +} + +function git(args: string[], cwd: string): string { + return execFileSync('git', args, { cwd, encoding: 'utf8', env: { ...process.env, ...GIT_ENV } }); +} + +const sandboxes: string[] = []; + +/** A learning file with frontmatter. */ +function learning(title: string, extra: string[] = []): string { + return ['---', `title: "${title}"`, 'date: 2026-09-20', 'confidence: 0.5', ...extra, '---', `# ${title}`, ''].join('\n'); +} + +/** + * A user-scope git install on a local bare team repo, with teamai-learnings + * and teamai-reports already on origin and one `teamai pull` done. With + * `project`, the install has that project active and it owns the learnings + * namespace of the same name. + */ +async function setUp(opts: { + project?: string; + mainLearnings?: Record<string, string>; + branchLearnings?: Record<string, string>; + votes?: string; +} = {}) { + const sandbox = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'teamai-issue823-remnants-'))); + sandboxes.push(sandbox); + const home = path.join(sandbox, 'home'); + const remote = path.join(sandbox, 'team.git'); + const clone = path.join(home, '.teamai', 'team-repo'); + const checkout = path.join(home, '.teamai', 'learnings-wt'); + fs.mkdirSync(path.join(home, '.claude'), { recursive: true }); + + const seed = path.join(sandbox, 'seed'); + fs.mkdirSync(seed); + fs.writeFileSync(path.join(seed, 'teamai.yaml'), 'team: issue-823\nrepo: https://github.com/acme/team\nprovider: github\n'); + if (opts.project) { + fs.mkdirSync(path.join(seed, 'manifest')); + fs.writeFileSync(path.join(seed, 'manifest', 'projects.yaml'), [ + 'version: 1', + 'projects:', + ` - id: ${opts.project}`, + ` name: ${opts.project}`, + ' resources:', + ` learnings: [${opts.project}]`, + '', + ].join('\n')); + } + for (const [name, content] of Object.entries(opts.mainLearnings ?? {})) { + fs.mkdirSync(path.join(seed, 'learnings'), { recursive: true }); + fs.writeFileSync(path.join(seed, 'learnings', name), content); + } + git(['init', '-q', '-b', 'main'], seed); + git(['add', '-A'], seed); + git(['commit', '-q', '-m', 'seed'], seed); + + git(['checkout', '-q', '--orphan', 'teamai-learnings'], seed); + git(['rm', '-rfq', '.'], seed); + fs.writeFileSync(path.join(seed, '.gitignore'), 'reports-wt/\nlearnings-wt/\nknowledge-wt/\n'); + fs.mkdirSync(path.join(seed, 'learnings'), { recursive: true }); + fs.writeFileSync(path.join(seed, 'learnings', '.gitkeep'), ''); + for (const [name, content] of Object.entries(opts.branchLearnings ?? {})) { + fs.mkdirSync(path.dirname(path.join(seed, 'learnings', name)), { recursive: true }); + fs.writeFileSync(path.join(seed, 'learnings', name), content); + } + git(['add', '-A'], seed); + git(['commit', '-q', '-m', 'learnings branch'], seed); + + git(['checkout', '-q', '--orphan', 'teamai-reports'], seed); + git(['rm', '-rfq', '.'], seed); + fs.writeFileSync(path.join(seed, '.gitignore'), 'reports-wt/\nlearnings-wt/\nknowledge-wt/\n'); + if (opts.votes) { + fs.mkdirSync(path.join(seed, 'votes')); + fs.writeFileSync(path.join(seed, 'votes', 'teammate.yaml'), opts.votes); + } + git(['add', '-A'], seed); + git(['commit', '-q', '-m', 'reports branch'], seed); + git(['checkout', '-q', 'main'], seed); + + git(['clone', '-q', '--bare', seed, remote], sandbox); + git(['clone', '-q', remote, clone], sandbox); + fs.writeFileSync(path.join(home, '.teamai', 'config.yaml'), [ + 'repo:', + ` localPath: ${clone}`, + ` remote: ${remote}`, + ' kind: git', + 'username: ci-823', + 'updatePolicy: auto', + 'scope: user', + 'enabledAgents: [claude]', + ...(opts.project ? [`projects: [${opts.project}]`] : []), + '', + ].join('\n')); + + const run = (args: string[]): Promise<RunResult> => new Promise((resolve) => { + const { CLAUDE_SESSION_ID: _s, GITHUB_TOKEN: _t, GH_TOKEN: _g, ...env } = process.env; + const child = spawn('node', [CLI, ...args], { + cwd: home, + env: { ...env, ...GIT_ENV, HOME: home, USERPROFILE: home, FORCE_COLOR: '0', NO_COLOR: '1', TEAMAI_CONTRIBUTE_HINT_DISABLED: '1' }, + stdio: ['ignore', 'pipe', 'pipe'], + }); + let output = ''; + child.stdout.on('data', (d: Buffer) => { output += d.toString(); }); + child.stderr.on('data', (d: Buffer) => { output += d.toString(); }); + child.on('close', (code) => resolve({ code, output })); + }); + + const pull = await run(['pull']); + expect(pull.code, pull.output).toBe(0); + expect(fs.existsSync(path.join(checkout, '.git')), pull.output).toBe(true); + + const published = (): string[] => + git(['ls-tree', '-r', '--name-only', 'teamai-learnings'], remote).split('\n').filter((f) => f.endsWith('.md')); + const show = (file: string): string => git(['show', `teamai-learnings:${file}`], remote); + const status = (): string => git(['status', '--porcelain', '--untracked-files=all'], checkout); + return { home, remote, checkout, run, published, show, status }; +} + +/** What an older import --from-mr wrote: `<date>-<title>.md`, with the MR in its frontmatter, never committed. */ +const REMNANT = [ + '---', + 'title: "Quokka cache warmup before deploy"', + 'author: dev', + 'date: 2026-09-20', + 'tags: [cache, deploy]', + 'confidence: 0.85', + 'source_mr: "https://github.com/acme/app/pull/42"', + '---', + '## Background', + 'The quokka cache is cold after each deploy.', + '', +].join('\n'); +const REMNANT_NAME = '2026-09-20-Quokka-cache-warmup-before-deploy.md'; + +describe('learnings the checkout holds but never committed (#823)', () => { + beforeAll(() => { + if (!fs.existsSync(CLI)) throw new Error(`CLI binary not found at ${CLI}. Run "npm run build" first.`); + }); + + afterEach(() => { + for (const sandbox of sandboxes.splice(0)) fs.rmSync(sandbox, { recursive: true, force: true }); + }); + + it("publishes an older import --from-mr learning on the next pull, in the project's namespace, and leaves the checkout clean (item 7)", async () => { + const s = await setUp({ project: 'alpha' }); + const file = path.join(s.checkout, 'learnings', REMNANT_NAME); + fs.writeFileSync(file, REMNANT); + + const pull = await s.run(['pull']); + + expect(pull.code, pull.output).toBe(0); + const queued = s.published().filter((f) => /^learnings\/alpha\/quokka-cache-warmup-before-deploy-\d{4}-\d{2}-\d{2}-[a-z0-9]+\.md$/.test(f)); + expect(queued, `${s.published().join('\n')}\n${pull.output}`).toHaveLength(1); + expect(s.show(queued[0])).toBe(REMNANT); + expect(s.status()).toBe(''); + expect(pull.output).toContain(`Queued 1 learning(s) an older teamai import --from-mr left unpublished: ${file}`); + }); + + it('publishes only what confidence write-back changed, never a file nobody committed (maintenance sweep)', async () => { + const recent = new Date().toISOString(); + const vote = (doc: string) => [ + ` ${doc}:`, + ' recalled_count: 9', + ' upvoted_count: 6', + ` last_recalled_at: ${recent}`, + ` last_upvoted_at: ${recent}`, + ]; + const s = await setUp({ + mainLearnings: { 'inherited-note.md': learning('Inherited note') }, + branchLearnings: { 'tracked-note.md': learning('Tracked note') }, + votes: ['version: 2', 'votes:', ...vote('tracked-note'), ...vote('inherited-note'), ''].join('\n'), + }); + // A draft nobody meant to share. It has no votes, so maintenance never touches it. + const stray = path.join(s.checkout, 'learnings', 'stray-draft.md'); + fs.writeFileSync(stray, learning('Stray draft')); + + const maintenance = await s.run(['recall', 'maintenance', '--confidence-writeback']); + + expect(maintenance.code, maintenance.output).toBe(0); + expect(maintenance.output).toContain('Published maintenance changes to the learnings branch'); + // The tracked learning it rewrote, and the new copy of the inherited one it wrote into the checkout. + expect(s.show('learnings/tracked-note.md')).not.toContain('confidence: 0.5'); + expect(s.show('learnings/inherited-note.md')).not.toContain('confidence: 0.5'); + expect(s.published()).not.toContain('learnings/stray-draft.md'); + expect(fs.readFileSync(stray, 'utf8')).toBe(learning('Stray draft')); + }); + + it('publishes an archived prune, and still not a file nobody committed (maintenance sweep)', async () => { + const s = await setUp({ + branchLearnings: { 'low-note.md': learning('Low note') }, + votes: ['version: 2', 'votes:', ' low-note:', ' recalled_count: 1', ' upvoted_count: 0', + ' last_recalled_at: 2020-01-01T00:00:00.000Z', ''].join('\n'), + }); + const stray = path.join(s.checkout, 'learnings', 'stray-draft.md'); + fs.writeFileSync(stray, learning('Stray draft')); + + const prune = await s.run(['recall', 'maintenance', '--prune', '--archive']); + + expect(prune.code, prune.output).toBe(0); + expect(prune.output).toContain('Published maintenance changes to the learnings branch'); + expect(s.published()).toContain('learnings/_archive/low-note.md'); + expect(s.published()).not.toContain('learnings/low-note.md'); + expect(s.published()).not.toContain('learnings/stray-draft.md'); + expect(fs.existsSync(stray)).toBe(true); + }); +}); + +describe('pull --dry-run and the contribution queue (#823 item 20)', () => { + beforeAll(() => { + if (!fs.existsSync(CLI)) throw new Error(`CLI binary not found at ${CLI}. Run "npm run build" first.`); + }); + + afterEach(() => { + for (const sandbox of sandboxes.splice(0)) fs.rmSync(sandbox, { recursive: true, force: true }); + }); + + it('says what it would publish and publishes nothing, leaving the learning queued for the next pull', async () => { + const s = await setUp(); + const note = path.join(s.home, 'otter.md'); + fs.writeFileSync(note, '# Otter note\nOtters hold hands while they sleep.\n'); + // Origin rejects the push, so the contribution stays queued. + const hook = path.join(s.remote, 'hooks', 'pre-receive'); + fs.writeFileSync(hook, '#!/bin/sh\necho "rejected by the #823 test" >&2\nexit 1\n', { mode: 0o755 }); + const contribute = await s.run(['contribute', '--title', 'otter', '--file', note]); + fs.rmSync(hook); + expect(contribute.output).toContain('Saved locally'); + const queue = path.join(s.home, '.teamai', 'pending-learnings'); + const queued = fs.readdirSync(queue).filter((f) => f.endsWith('.md')); + expect(queued, contribute.output).toHaveLength(1); + const before = s.published(); + + const dryRun = await s.run(['pull', '--dry-run']); + + expect(dryRun.code, dryRun.output).toBe(0); + expect(s.published(), dryRun.output).toEqual(before); + expect(fs.readdirSync(queue).filter((f) => f.endsWith('.md'))).toEqual(queued); + expect(dryRun.output).toContain('[dry-run] Would publish 1 queued learning(s)'); + expect(dryRun.output).not.toContain('Published 1 queued learning(s)'); + expect(dryRun.output).not.toContain('written locally but not published'); + + const pull = await s.run(['pull']); + + expect(pull.output).toContain('Published 1 queued learning(s)'); + expect(s.published()).toContain(`learnings/${queued[0]}`); + }); +}); diff --git a/src/__tests__/e2e/self-mode-worktrees-808.test.ts b/src/__tests__/e2e/self-mode-worktrees-808.test.ts index 8ef3f4fe1..a7c3691be 100644 --- a/src/__tests__/e2e/self-mode-worktrees-808.test.ts +++ b/src/__tests__/e2e/self-mode-worktrees-808.test.ts @@ -35,11 +35,11 @@ interface RunResult { output: string; } -function runCLI(args: string[], cwd: string, home: string): Promise<RunResult> { +function runCLI(args: string[], cwd: string, home: string, env: NodeJS.ProcessEnv = {}): Promise<RunResult> { return new Promise((resolve) => { const child = spawn('node', [CLI, ...args], { cwd, - env: { ...process.env, ...GIT_ENV, HOME: home, FORCE_COLOR: '0' }, + env: { ...process.env, ...GIT_ENV, HOME: home, FORCE_COLOR: '0', ...env }, stdio: ['ignore', 'pipe', 'pipe'], }); let output = ''; @@ -368,6 +368,55 @@ describe('self mode with a linked worktree (#808)', () => { expect(published(project).some((f) => f.includes('unmigrated-note-')), published(project).join('\n')).toBe(true); }); + it('moves the queue an older teamai left in a linked worktree before import --from-mr publishes, so removing the worktree loses nothing', async () => { + const project = setUpProject(); + const { home, projectRoot, worktree } = project; + // The main checkout migrated; the worktree still holds the queue an older + // teamai kept in its own .teamai/, with no config beside it. + expect((await runCLI(['pull'], projectRoot, home)).code).toBe(0); + git(['worktree', 'add', '-q', worktree, '-b', 'wt-808'], projectRoot); + const legacy = 'legacy-wt-2026-01-01-bbbbbb.md'; + fs.mkdirSync(path.join(worktree, '.teamai', 'pending-learnings'), { recursive: true }); + fs.writeFileSync(path.join(worktree, '.teamai', 'pending-learnings', legacy), '# Queued in the worktree by an older teamai\n'); + // `gh` (the MR) and `claude` (the extraction) are stand-ins on PATH. + const bin = path.join(project.sandbox, 'bin'); + fs.mkdirSync(bin); + fs.writeFileSync(path.join(bin, 'gh'), [ + '#!/usr/bin/env bash', + 'case "$1 $2" in', + ` "pr view") echo '{"title":"Retry flaky upload","body":"Retries uploads.","author":{"login":"dev"},"mergedAt":"2026-09-20T10:00:00Z","commits":[]}' ;;`, + " \"pr diff\") printf 'diff --git a/up.ts b/up.ts\\n+retry(3)\\n' ;;", + ' *) exit 1 ;;', + 'esac', + '', + ].join('\n'), { mode: 0o755 }); + fs.writeFileSync(path.join(bin, 'claude'), [ + '#!/usr/bin/env bash', + "cat <<'MD'", + '---', + 'title: Retry flaky uploads', + '---', + '# Retry flaky uploads', + '', + 'Wrap uploads in retry(3).', + 'MD', + '', + ].join('\n'), { mode: 0o755 }); + + const imported = await runCLI( + ['import', '--from-mr', 'https://github.com/acme/app/pull/7', '--all'], + worktree, + home, + { PATH: `${bin}${path.delimiter}${process.env.PATH ?? ''}`, CLAUDE_SESSION_ID: '', GITHUB_TOKEN: '', GH_TOKEN: '' }, + ); + + expect(imported.code, imported.output).toBe(0); + git(['worktree', 'remove', '--force', worktree], projectRoot); + expect(published(project), `${published(project).join('\n')}\n${imported.output}`).toContain(`learnings/${legacy}`); + expect(published(project).some((f) => f.includes('retry-flaky-uploads-')), published(project).join('\n')).toBe(true); + expect(partitionQueue(project)).toEqual([]); + }); + it('stops contribute in an unmigrated linked worktree while another command holds its sync lock, saving nothing there', async () => { const project = setUpProject(); const { home, projectRoot, worktree } = project; @@ -639,6 +688,107 @@ describe('self mode with a linked worktree (#808)', () => { expect(published(project)).toContain('learnings/legacy-wt-2026-01-01-bbbbbb.md'); }); + it("publishes the learning an older import --from-mr left in the old checkout, so git can remove it and contribute publishes both (#823 item 7)", async () => { + const project = setUpProject(); + const { home, remote, projectRoot } = project; + const seed = path.join(project.sandbox, 'seed'); + git(['clone', '-q', remote, seed], project.sandbox); + git(['checkout', '-q', '--orphan', 'teamai-learnings'], seed); + git(['rm', '-rfq', '.'], seed); + fs.mkdirSync(path.join(seed, 'learnings'), { recursive: true }); + fs.writeFileSync(path.join(seed, '.gitignore'), 'reports-wt/\nlearnings-wt/\nknowledge-wt/\n'); + fs.writeFileSync(path.join(seed, 'learnings', '.gitkeep'), ''); + git(['add', '-A'], seed); + git(['commit', '-q', '-m', 'learnings branch'], seed); + git(['push', '-q', 'origin', 'teamai-learnings'], seed); + git(['fetch', '-q', 'origin', 'teamai-learnings:teamai-learnings'], projectRoot); + // The checkout an older teamai kept in .teamai/, holding what an older + // import --from-mr wrote there and never committed. + const oldCheckout = path.join(projectRoot, '.teamai', 'learnings-wt'); + git(['worktree', 'add', '-q', oldCheckout, 'teamai-learnings'], projectRoot); + const remnant = [ + '---', + 'title: "Quokka cache warmup before deploy"', + 'date: 2026-09-20', + 'source_mr: "https://github.com/acme/app/pull/42"', + '---', + 'Warm the quokka cache in the post-deploy hook.', + '', + ].join('\n'); + fs.writeFileSync(path.join(oldCheckout, 'learnings', '2026-09-20-Quokka-cache-warmup-before-deploy.md'), remnant); + + const contribute = await runCLI( + ['contribute', '--title', 'self-note', '--file', note(project, 'self', 'Note contributed beside the old checkout')], + projectRoot, + home, + ); + + expect(contribute.code, contribute.output).toBe(0); + expect(contribute.output).not.toContain('still has teamai-learnings checked out'); + expect(contribute.output).not.toContain('Saved locally'); + expect(fs.existsSync(oldCheckout)).toBe(false); + expect(published(project).some((f) => f.includes('self-note-')), published(project).join('\n')).toBe(true); + const quokka = published(project).filter((f) => /^learnings\/quokka-cache-warmup-before-deploy-[\d-]+-[a-z0-9]+\.md$/.test(f)); + expect(quokka, published(project).join('\n')).toHaveLength(1); + expect(git(['show', `teamai-learnings:${quokka[0]}`], remote)).toBe(remnant); + expect(partitionQueue(project)).toEqual([]); + }); + + it("removes that learning instead when a teammate already published one from the same merge request the old checkout never fetched (#823 item 21)", async () => { + const project = setUpProject(); + const { home, remote, projectRoot } = project; + const seed = path.join(project.sandbox, 'seed'); + git(['clone', '-q', remote, seed], project.sandbox); + git(['checkout', '-q', '--orphan', 'teamai-learnings'], seed); + git(['rm', '-rfq', '.'], seed); + fs.mkdirSync(path.join(seed, 'learnings'), { recursive: true }); + fs.writeFileSync(path.join(seed, '.gitignore'), 'reports-wt/\nlearnings-wt/\nknowledge-wt/\n'); + fs.writeFileSync(path.join(seed, 'learnings', '.gitkeep'), ''); + git(['add', '-A'], seed); + git(['commit', '-q', '-m', 'learnings branch'], seed); + git(['push', '-q', 'origin', 'teamai-learnings'], seed); + git(['fetch', '-q', 'origin', 'teamai-learnings:teamai-learnings'], projectRoot); + const oldCheckout = path.join(projectRoot, '.teamai', 'learnings-wt'); + git(['worktree', 'add', '-q', oldCheckout, 'teamai-learnings'], projectRoot); + const remnantFile = path.join(oldCheckout, 'learnings', '2026-09-20-Quokka-cache-warmup-before-deploy.md'); + fs.writeFileSync(remnantFile, [ + '---', + 'title: "Quokka cache warmup before deploy"', + 'date: 2026-09-20', + 'source_mr: "https://github.com/acme/app/pull/42"', + '---', + 'Warm the quokka cache in the post-deploy hook.', + '', + ].join('\n')); + // A teammate's later import of the same MR, pushed after this repo last fetched. + const teammates = 'learnings/quokka-cache-warmup-2026-09-21-ttt111.md'; + fs.writeFileSync(path.join(seed, teammates), [ + '---', + 'title: "Quokka cache warmup"', + 'date: 2026-09-21', + 'source_mr: "https://github.com/acme/app/pull/42"', + '---', + 'Warm the quokka cache before traffic returns.', + '', + ].join('\n')); + git(['add', '-A'], seed); + git(['commit', '-q', '-m', 'teammate import'], seed); + git(['push', '-q', 'origin', 'teamai-learnings'], seed); + + const contribute = await runCLI( + ['contribute', '--title', 'self-note', '--file', note(project, 'self', 'Note contributed beside the old checkout')], + projectRoot, + home, + ); + + expect(contribute.code, contribute.output).toBe(0); + expect(contribute.output).toContain(`Removed ${remnantFile}, which an older teamai import --from-mr left unpublished: ${teammates} already has it.`); + expect(fs.existsSync(oldCheckout)).toBe(false); + expect(published(project).filter((f) => f.includes('quokka')), published(project).join('\n')).toEqual([teammates]); + expect(published(project).some((f) => f.includes('self-note-')), published(project).join('\n')).toBe(true); + expect(partitionQueue(project)).toEqual([]); + }); + it("refuses a learnings checkout another repository left in the partition, and publishes nothing to it", async () => { const project = setUpProject(); const { sandbox, home, projectRoot } = project; @@ -1372,6 +1522,147 @@ describe('a checkout an older git-mode install left (#808)', () => { expect(git(['log', '--all', '--name-only', '--format='], teamB.bare)).not.toContain('team-a-note-'); }); + it("leaves no config naming team A beside team B's clone when init stops after recloning, and sets team A's queue aside (#823 item 17)", async () => { + const install = setUpGitInstall(); + const { home, projectRoot } = install; + const teamA = serveTeamRepo(install, 'team-a'); + const teamB = serveTeamRepo(install, 'team-b'); + // Team B has roles, so an unknown --role stops init after it has recloned. + const edit = path.join(install.sandbox, 'team-b-edit'); + git(['clone', '-q', teamB.bare, edit], install.sandbox); + fs.mkdirSync(path.join(edit, 'manifest')); + fs.writeFileSync(path.join(edit, 'manifest', 'roles.yaml'), 'version: 1\nroles:\n - id: backend\n resources:\n knowledge: []\n skills: []\n'); + git(['add', '-A'], edit); + git(['commit', '-q', '-m', 'roles'], edit); + git(['push', '-q', 'origin', 'main'], edit); + + const initA = await runCLI(['init', teamA.url, '--scope', 'project', '--force'], projectRoot, home); + expect(initA.code, initA.output).toBe(0); + const queued = await withPushesRejected({ ...install, remote: teamA.bare }, () => runCLI( + ['contribute', '--title', 'team-a-note', '--file', note(install, 'team-a', 'Note for team A')], + projectRoot, + home, + )); + expect(queued.output).toContain('Saved locally'); + const [name] = partitionQueue(install).filter((f) => f.startsWith('team-a-note-')); + expect(name, partitionQueue(install).join('\n')).toBeDefined(); + + const failed = await runCLI(['init', teamB.url, '--scope', 'project', '--force', '--role', 'ghost'], projectRoot, home); + + expect(failed.code, failed.output).toBe(1); + expect(failed.output).toContain('Unknown role "ghost"'); + const partition = partitionOf(install); + expect(git(['remote', 'get-url', 'origin'], path.join(partition, 'team-repo')).trim()).toBe(teamB.url); + const config = path.join(partition, 'config.yaml'); + expect(fs.existsSync(config) ? fs.readFileSync(config, 'utf8') : '', failed.output).not.toContain(teamA.url); + // Moved aside, not deleted, and the member is told where. + expect(fs.readFileSync(`${config}.previous`, 'utf8')).toContain(teamA.url); + expect(failed.output).toContain(`Moved ${config} to ${config}.previous`); + const aside = path.join(partition, 'pending-learnings.git-git.example.com-team-team-a'); + expect(fs.readFileSync(path.join(aside, name), 'utf8')).toContain('Note for team A'); + expect(partitionQueue(install)).not.toContain(name); + + const initB = await runCLI(['init', teamB.url, '--scope', 'project', '--force', '--role', 'backend'], projectRoot, home); + expect(initB.code, initB.output).toBe(0); + const pull = await runCLI(['pull'], projectRoot, home); + expect(pull.code, pull.output).toBe(0); + expect(git(['log', '--all', '--name-only', '--format='], teamB.bare)).not.toContain('team-a-note-'); + }); + + it("moves team A's config aside when init reuses a clone of team B an earlier init left beside it and then fails, and carries its settings on the rerun (#823 item 17)", async () => { + const install = setUpGitInstall(); + const { home, projectRoot } = install; + const teamA = serveTeamRepo(install, 'team-a'); + const teamB = serveTeamRepo(install, 'team-b'); + + const initA = await runCLI(['init', teamA.url, '--scope', 'project', '--force', '--agent', 'claude'], projectRoot, home); + expect(initA.code, initA.output).toBe(0); + const partition = partitionOf(install); + const config = path.join(partition, 'config.yaml'); + // What an init that recloned team B and stopped before saving left: team A's config beside team B's clone. + const clone = path.join(partition, 'team-repo'); + fs.rmSync(clone, { recursive: true, force: true }); + git(['clone', '-q', teamB.bare, clone], install.sandbox); + git(['remote', 'set-url', 'origin', teamB.url], clone); + // Without team B's rewrite the clone names team B itself, so init reuses it, + // and with HTTPS refused its refresh fails. + const gitconfig = path.join(home, '.gitconfig'); + const rewrites = fs.readFileSync(gitconfig, 'utf8'); + fs.writeFileSync(gitconfig, rewrites.replace(`[url "${teamB.bare}"]\n\tinsteadOf = ${teamB.url}\n`, '')); + + const failed = await runCLI(['init', teamB.url, '--scope', 'project', '--force'], projectRoot, home, { GIT_ALLOW_PROTOCOL: 'file' }); + + expect(failed.code, failed.output).toBe(1); + expect(failed.output).toContain('using existing clone'); + expect(failed.output).toContain('Failed to refresh existing clone'); + expect(fs.existsSync(config) ? fs.readFileSync(config, 'utf8') : '', failed.output).not.toContain(teamA.url); + expect(fs.readFileSync(`${config}.previous`, 'utf8')).toContain(teamA.url); + fs.writeFileSync(gitconfig, rewrites); + + const initB = await runCLI(['init', teamB.url, '--scope', 'project', '--force'], projectRoot, home); + + expect(initB.code, initB.output).toBe(0); + const saved = fs.readFileSync(config, 'utf8'); + expect(saved).toContain(teamB.url); + expect(saved).toMatch(/enabledAgents:\n\s+- claude\n/); + }); + + it("carries the set-aside config's settings into the init that reruns after the replacement clone failed (#823 item 17)", async () => { + const install = setUpGitInstall(); + const { home, projectRoot } = install; + const teamA = serveTeamRepo(install, 'team-a'); + const teamB = serveTeamRepo(install, 'team-b'); + + const initA = await runCLI(['init', teamA.url, '--scope', 'project', '--force', '--agent', 'claude'], projectRoot, home); + expect(initA.code, initA.output).toBe(0); + const config = path.join(partitionOf(install), 'config.yaml'); + expect(fs.readFileSync(config, 'utf8')).toMatch(/enabledAgents:\n\s+- claude\n/); + + // Team B is unreachable, so the clone fails after the config was moved aside. + fs.renameSync(teamB.bare, `${teamB.bare}.offline`); + const failed = await runCLI(['init', teamB.url, '--scope', 'project', '--force'], projectRoot, home); + expect(failed.code, failed.output).toBe(1); + expect(failed.output).toContain('Clone failed'); + expect(fs.existsSync(config)).toBe(false); + fs.renameSync(`${teamB.bare}.offline`, teamB.bare); + + const initB = await runCLI(['init', teamB.url, '--scope', 'project', '--force', '--agent', 'codex'], projectRoot, home); + + expect(initB.code, initB.output).toBe(0); + const saved = fs.readFileSync(config, 'utf8'); + expect(saved).toContain(teamB.url); + expect(saved).toMatch(/enabledAgents:\n\s+- claude\n\s+- codex\n/); + }); + + it('keeps the agent lists of the set-aside config when the rerun after a failed replacement clone names no --agent (#823 item 17)', async () => { + const install = setUpGitInstall(); + const { home, projectRoot } = install; + const teamA = serveTeamRepo(install, 'team-a'); + const teamB = serveTeamRepo(install, 'team-b'); + + const initA = await runCLI(['init', teamA.url, '--scope', 'project', '--force', '--agent', 'claude'], projectRoot, home); + expect(initA.code, initA.output).toBe(0); + const config = path.join(partitionOf(install), 'config.yaml'); + // What `uninstall --agent codex` records. + const written = fs.readFileSync(config, 'utf8'); + expect(written).toContain('disabledAgents: []\n'); + fs.writeFileSync(config, written.replace('disabledAgents: []\n', 'disabledAgents:\n - codex\n')); + + fs.renameSync(teamB.bare, `${teamB.bare}.offline`); + const failed = await runCLI(['init', teamB.url, '--scope', 'project', '--force'], projectRoot, home); + expect(failed.code, failed.output).toBe(1); + expect(fs.existsSync(config)).toBe(false); + fs.renameSync(`${teamB.bare}.offline`, teamB.bare); + + const initB = await runCLI(['init', teamB.url, '--scope', 'project', '--force'], projectRoot, home); + + expect(initB.code, initB.output).toBe(0); + const saved = fs.readFileSync(config, 'utf8'); + expect(saved).toContain(teamB.url); + expect(saved).toMatch(/enabledAgents:\n\s+- claude\n/); + expect(saved).toMatch(/disabledAgents:\n\s+- codex\n/); + }); + it("sets aside the queue under an unknown owner when init replaces a config it cannot read, and the next pull publishes none of it", async () => { const install = setUpGitInstall(); const { home, projectRoot } = install; diff --git a/src/__tests__/import-mr-learnings-checkout.test.ts b/src/__tests__/import-mr-learnings-checkout.test.ts index 9b3b43400..5d2fd4f21 100644 --- a/src/__tests__/import-mr-learnings-checkout.test.ts +++ b/src/__tests__/import-mr-learnings-checkout.test.ts @@ -26,12 +26,9 @@ const localConfig = { dataHome, }; -/** The config teamai detects: the main checkout unless a test runs in a linked worktree. */ -let detected = localConfig; - vi.mock('../config.js', async (importOriginal) => ({ ...(await importOriginal<typeof import('../config.js')>()), - autoDetectInit: vi.fn(async () => ({ localConfig: detected, teamConfig: { team: 't', repo: remote } })), + autoDetectInit: vi.fn(async () => ({ localConfig, teamConfig: { team: 't', repo: remote } })), })); const IMPORTED = 'mr-note-2026-01-01-aaaaaa.md'; @@ -99,7 +96,6 @@ describe('import --from-mr in self mode (#808)', () => { }); afterEach(() => { - detected = localConfig; fs.rmSync(testRoot, { recursive: true, force: true }); }); @@ -114,26 +110,6 @@ describe('import --from-mr in self mode (#808)', () => { expect(fs.existsSync(path.join(pendingLearningsDir(localConfig), IMPORTED))).toBe(false); }); - it('publishes what an older teamai queued in a linked worktree, so removing the worktree loses nothing', async () => { - const worktree = path.join(testRoot, 'business-wt'); - git(['worktree', 'add', '-q', worktree, '-b', 'wt-808'], businessRoot); - const legacy = 'legacy-wt-2026-01-01-bbbbbb.md'; - const legacyQueue = path.join(worktree, '.teamai', 'pending-learnings'); - fs.mkdirSync(legacyQueue, { recursive: true }); - fs.writeFileSync(path.join(legacyQueue, legacy), '# Queued in the worktree by an older teamai\n'); - detected = { - ...localConfig, - repo: { ...localConfig.repo, localPath: path.join(worktree, '.teamai'), businessRepoRoot: worktree }, - projectRoot: worktree, - }; - - await importCmd({ fromMr: 'https://github.com/o/r/pull/1' }); - - git(['worktree', 'remove', '--force', worktree], businessRoot); - expect(publishedLearnings()).toEqual(expect.arrayContaining([`learnings/${legacy}`, `learnings/${IMPORTED}`])); - expect(fs.readdirSync(pendingLearningsDir(localConfig))).toEqual([]); - }); - it('leaves the learnings checkout alone when the learning goes to --output', async () => { await importCmd({ fromMr: 'https://github.com/o/r/pull/1', output: path.join(testRoot, 'drafts') }); diff --git a/src/__tests__/learnings-import-remnants.test.ts b/src/__tests__/learnings-import-remnants.test.ts new file mode 100644 index 000000000..97124e8e4 --- /dev/null +++ b/src/__tests__/learnings-import-remnants.test.ts @@ -0,0 +1,731 @@ +/** + * Learnings an older `import --from-mr` (0.25.0 to 0.26.0-beta.3) wrote into the learnings + * checkout and never committed (#823 item 7). They were recallable on this + * machine, never reached the team, and in single-repo mode they keep git from + * removing the old checkout. Publishing now moves them into the queue first. + * + * Real git, no mocks of the units under test, as in git-kind-learnings.test.ts. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { simpleGit } from 'simple-git'; + +import { getDataHome, LEARNINGS_LOCK_FILENAME, SYNC_LOCK_FILENAME, type LocalConfig } from '../types.js'; +import { learningsBranch } from '../utils/learnings-branch.js'; +import { listPendingLearnings, savePendingLearning } from '../utils/pending-learnings.js'; +import { publishLearningsMaintenance, publishQueuedLearnings } from '../utils/learnings-publish.js'; +import { log } from '../utils/logger.js'; +import { writeInstallConfig } from './helpers/install-config.js'; + +let tmp: string; +let originalHome: string; + +beforeEach(() => { + // Real path: git lists worktrees by it, and the warnings name those paths. + tmp = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'teamai-import-remnants-'))); + originalHome = process.env.HOME ?? ''; + process.env.HOME = path.join(tmp, 'home'); + fs.mkdirSync(process.env.HOME, { recursive: true }); +}); + +afterEach(() => { + vi.restoreAllMocks(); + process.env.HOME = originalHome; + fs.rmSync(tmp, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); +}); + +async function configureGit(dir: string): Promise<void> { + const git = simpleGit(dir); + await git.addConfig('user.email', 't@t.com'); + await git.addConfig('user.name', 't'); +} + +/** A team repo, a member's clone, and the learnings checkout a first publish created. */ +async function setUp(): Promise<{ config: LocalConfig; origin: string; checkout: string }> { + const seed = path.join(tmp, 'seed'); + fs.mkdirSync(seed, { recursive: true }); + const seedGit = simpleGit(seed); + await seedGit.init(['--initial-branch=main']); + await configureGit(seed); + fs.writeFileSync(path.join(seed, 'teamai.yaml'), 'team: acme\n'); + await seedGit.add(['.']); + await seedGit.commit('init'); + const origin = path.join(tmp, 'origin.git'); + await simpleGit().clone(seed, origin, ['--bare']); + const clone = path.join(tmp, 'team-repo'); + await simpleGit().clone(origin, clone); + await configureGit(clone); + + const config: LocalConfig = { + repo: { localPath: clone, remote: origin, kind: 'git' }, + username: 'alice', + scope: 'user', + additionalRoles: [], + }; + writeInstallConfig(config); + const checkout = await learningsBranch.ensure(config); + await configureGit(checkout); + return { config, origin, checkout }; +} + +async function publishedFiles(origin: string): Promise<string[]> { + try { + return (await simpleGit(origin).raw(['ls-tree', '-r', '--name-only', 'teamai-learnings'])).split('\n').filter(Boolean); + } catch { + return []; + } +} + +async function publishedContent(origin: string, file: string): Promise<string> { + return simpleGit(origin).raw(['show', `teamai-learnings:${file}`]); +} + +/** What an older import --from-mr wrote: `<date>-<title>.md` with the MR in its frontmatter. */ +function remnant(mr = 'https://github.com/acme/app/pull/42', title = 'Quokka cache warmup before deploy'): string { + return [ + '---', + `title: "${title}"`, + 'author: dev', + 'date: 2026-09-20', + 'tags: [cache, deploy]', + 'confidence: 0.85', + `source_mr: "${mr}"`, + '---', + '## Background', + 'The quokka cache is cold after each deploy.', + '', + ].join('\n'); +} + +function plant(checkout: string, relPath: string, content: string): string { + const file = path.join(checkout, 'learnings', relPath); + fs.mkdirSync(path.dirname(file), { recursive: true }); + fs.writeFileSync(file, content); + return file; +} + +/** Run `fn` with origin unreachable, as when the member is offline. */ +async function offline<T>(origin: string, fn: () => Promise<T>): Promise<T> { + fs.renameSync(origin, `${origin}.offline`); + try { + return await fn(); + } finally { + fs.renameSync(`${origin}.offline`, origin); + } +} + +/** Hold the learnings branch lock, as a publish running in another command does. */ +function holdLearningsLock(config: LocalConfig): () => void { + const lock = path.join(path.dirname(learningsBranch.dir(config)), LEARNINGS_LOCK_FILENAME); + fs.writeFileSync(lock, JSON.stringify({ pid: process.pid, startedAt: new Date().toISOString(), owner: 'a publish' })); + return () => fs.rmSync(lock, { force: true }); +} + +/** A teammate publishes `rel` under `learnings/` from their own clone. */ +async function teammatePublishes(origin: string, rel: string, content: string): Promise<void> { + const teammate = path.join(tmp, 'teammate'); + await simpleGit().clone(origin, teammate, ['--branch', 'teamai-learnings']); + await configureGit(teammate); + fs.mkdirSync(path.dirname(path.join(teammate, 'learnings', rel)), { recursive: true }); + fs.writeFileSync(path.join(teammate, 'learnings', rel), content); + await simpleGit(teammate).add(['.']); + await simpleGit(teammate).commit('teammate'); + await simpleGit(teammate).push('origin', 'teamai-learnings'); +} + +/** What was warned from now on, one line per call. */ +function watchWarnings(): () => string { + const warn = vi.spyOn(log, 'warn'); + return () => warn.mock.calls.map((c) => String(c[0])).join('\n'); +} + +const QUEUED_NAME = /^learnings\/quokka-cache-warmup-before-deploy-\d{4}-\d{2}-\d{2}-[a-z0-9]+\.md$/; + +describe('a learning an older import --from-mr left untracked in the learnings checkout (#823 item 7)', () => { + it('is queued and published even when nothing else is queued, and leaves the checkout clean', async () => { + const { config, origin, checkout } = await setUp(); + const file = plant(checkout, '2026-09-20-Quokka-cache-warmup-before-deploy.md', remnant()); + const warned = watchWarnings(); + + const report = await publishQueuedLearnings(config, 'alice'); + + const published = (await publishedFiles(origin)).filter((f) => QUEUED_NAME.test(f)); + expect(published, (await publishedFiles(origin)).join('\n')).toHaveLength(1); + expect(await publishedContent(origin, published[0])).toBe(remnant()); + expect(report.published).toHaveLength(1); + expect(fs.existsSync(file)).toBe(false); + expect(await simpleGit(checkout).raw(['status', '--porcelain'])).toBe(''); + expect(await listPendingLearnings(config)).toEqual([]); + expect(warned()).toContain(`Queued 1 learning(s) an older teamai import --from-mr left unpublished: ${file}`); + }); + + it('is queued too when its title kept no character in the file name', async () => { + const { config, origin, checkout } = await setUp(); + // A Cyrillic title: that import replaced every character, leaving `<date>-.md`. + const file = plant(checkout, '2026-09-20-.md', remnant('https://github.com/acme/app/pull/5', 'Прогрев кэша')); + + await publishQueuedLearnings(config, 'alice'); + + expect(fs.existsSync(file)).toBe(false); + const published = (await publishedFiles(origin)).filter((f) => f.endsWith('.md')); + expect(published).toHaveLength(1); + expect(await publishedContent(origin, published[0])).toBe(remnant('https://github.com/acme/app/pull/5', 'Прогрев кэша')); + }); + + it('leaves every other file alone: tracked edits, subdirectories, and files without the date name or source_mr', async () => { + const { config, origin, checkout } = await setUp(); + // A tracked learning the member edited by hand. + await savePendingLearning(config, '2026-09-19-tracked.md', remnant('https://github.com/acme/app/pull/1', 'Tracked')); + await publishQueuedLearnings(config, 'alice'); + const tracked = path.join(checkout, 'learnings', '2026-09-19-tracked.md'); + fs.appendFileSync(tracked, 'An edit nobody committed.\n'); + const kept = [ + plant(checkout, 'alpha/2026-09-20-in-a-namespace.md', remnant('https://github.com/acme/app/pull/2')), + plant(checkout, '2026-09-20-no-merge-request.md', '---\ntitle: No MR\n---\nA draft.\n'), + plant(checkout, 'notes-with-a-merge-request.md', remnant('https://github.com/acme/app/pull/3')), + ]; + const before = await publishedFiles(origin); + + await publishQueuedLearnings(config, 'alice'); + + expect(await publishedFiles(origin)).toEqual(before); + for (const file of kept) expect(fs.existsSync(file), file).toBe(true); + expect(fs.readFileSync(tracked, 'utf8')).toContain('An edit nobody committed.'); + expect(await listPendingLearnings(config)).toEqual([]); + }); + + it('is removed, not queued again, when the branch already has a learning from the same merge request', async () => { + const { config, origin, checkout } = await setUp(); + // A later import of the same MR, published in a project namespace. + await savePendingLearning(config, 'alpha/quokka-2026-09-21-aaa111.md', remnant('https://github.com/acme/app/pull/42', 'Quokka, reimported')); + await publishQueuedLearnings(config, 'alice'); + const file = plant(checkout, '2026-09-20-Quokka-cache-warmup-before-deploy.md', remnant()); + const before = await publishedFiles(origin); + const warned = watchWarnings(); + + await publishQueuedLearnings(config, 'alice'); + + expect(await publishedFiles(origin)).toEqual(before); + expect(fs.existsSync(file)).toBe(false); + expect(await listPendingLearnings(config)).toEqual([]); + expect(warned()).toContain(file); + expect(warned()).toContain('learnings/alpha/quokka-2026-09-21-aaa111.md'); + }); + + it('is removed, not queued again, when a teammate published one from the same merge request that the checkout has not fetched yet (item 21)', async () => { + const { config, origin, checkout } = await setUp(); + await savePendingLearning(config, 'first-2026-09-19-aaa000.md', '---\ntitle: First\n---\nFirst.\n'); + await publishQueuedLearnings(config, 'alice'); + // A teammate imports the same MR from their own clone. Neither this + // checkout nor this clone's origin/teamai-learnings has seen it. + const teammate = path.join(tmp, 'teammate'); + await simpleGit().clone(origin, teammate, ['--branch', 'teamai-learnings']); + await configureGit(teammate); + fs.mkdirSync(path.join(teammate, 'learnings', 'alpha'), { recursive: true }); + fs.writeFileSync(path.join(teammate, 'learnings', 'alpha', 'quokka-2026-09-21-ddd444.md'), remnant('https://github.com/acme/app/pull/42', 'Quokka, by a teammate')); + await simpleGit(teammate).add(['.']); + await simpleGit(teammate).commit('teammate'); + await simpleGit(teammate).push('origin', 'teamai-learnings'); + const file = plant(checkout, '2026-09-20-Quokka-cache-warmup-before-deploy.md', remnant()); + const before = await publishedFiles(origin); + const warned = watchWarnings(); + + await publishQueuedLearnings(config, 'alice'); + + expect(await publishedFiles(origin)).toEqual(before); + expect(fs.existsSync(file)).toBe(false); + expect(await listPendingLearnings(config)).toEqual([]); + expect(warned()).toContain('learnings/alpha/quokka-2026-09-21-ddd444.md'); + }); + + it('is queued and published, not removed, when origin has since deleted the learning from the same merge request that the checkout still tracks', async () => { + const { config, origin, checkout } = await setUp(); + await savePendingLearning(config, 'alpha/quokka-2026-09-21-aaa111.md', remnant('https://github.com/acme/app/pull/42', 'Quokka, reimported')); + await publishQueuedLearnings(config, 'alice'); + // A teammate deletes it on origin; this checkout still tracks its old copy. + const teammate = path.join(tmp, 'teammate'); + await simpleGit().clone(origin, teammate, ['--branch', 'teamai-learnings']); + await configureGit(teammate); + await simpleGit(teammate).rm(['learnings/alpha/quokka-2026-09-21-aaa111.md']); + await simpleGit(teammate).commit('teammate prunes'); + await simpleGit(teammate).push('origin', 'teamai-learnings'); + const file = plant(checkout, '2026-09-20-Quokka-cache-warmup-before-deploy.md', remnant()); + + await publishQueuedLearnings(config, 'alice'); + + expect(fs.existsSync(file)).toBe(false); + const published = (await publishedFiles(origin)).filter((f) => QUEUED_NAME.test(f)); + expect(published, (await publishedFiles(origin)).join('\n')).toHaveLength(1); + expect(await publishedContent(origin, published[0])).toBe(remnant()); + expect(await listPendingLearnings(config)).toEqual([]); + }); + + it('is kept, not queued, while origin cannot be fetched, so a teammate\'s import of the same merge request is still seen', async () => { + const { config, origin, checkout } = await setUp(); + await savePendingLearning(config, 'first-2026-09-19-aaa000.md', '---\ntitle: First\n---\nFirst.\n'); + await publishQueuedLearnings(config, 'alice'); + await teammatePublishes(origin, 'alpha/quokka-2026-09-21-ddd444.md', remnant('https://github.com/acme/app/pull/42', 'Quokka, by a teammate')); + const file = plant(checkout, '2026-09-20-Quokka-cache-warmup-before-deploy.md', remnant()); + + await offline(origin, () => publishQueuedLearnings(config, 'alice')); + + expect(fs.existsSync(file)).toBe(true); + expect(await listPendingLearnings(config)).toEqual([]); + + const before = await publishedFiles(origin); + await publishQueuedLearnings(config, 'alice'); + + expect(await publishedFiles(origin)).toEqual(before); + expect(fs.existsSync(file)).toBe(false); + expect(await listPendingLearnings(config)).toEqual([]); + }); + + it('is queued and published when origin has no learnings branch, as after an offline first publish', async () => { + const { config, origin, checkout } = await setUp(); + await simpleGit(origin).raw(['branch', '-D', 'teamai-learnings']); + const file = plant(checkout, '2026-09-20-Quokka-cache-warmup-before-deploy.md', remnant()); + + await publishQueuedLearnings(config, 'alice'); + + expect(fs.existsSync(file)).toBe(false); + expect((await publishedFiles(origin)).filter((f) => QUEUED_NAME.test(f))).toHaveLength(1); + expect(await listPendingLearnings(config)).toEqual([]); + }); + + it('is queued even when another file of the same shape cannot be read', async () => { + const { config, origin, checkout } = await setUp(); + const file = plant(checkout, '2026-09-20-Quokka-cache-warmup-before-deploy.md', remnant()); + fs.symlinkSync(path.join(tmp, 'nowhere.md'), path.join(checkout, 'learnings', '2026-09-19-Broken.md')); + + await publishQueuedLearnings(config, 'alice'); + + expect(fs.existsSync(file)).toBe(false); + expect((await publishedFiles(origin)).filter((f) => QUEUED_NAME.test(f))).toHaveLength(1); + }); + + it('is removed, and published once, when the queue already holds the same content', async () => { + const { config, origin, checkout } = await setUp(); + await savePendingLearning(config, 'quokka-copy-2026-09-21-bbb222.md', remnant('https://github.com/acme/app/pull/77')); + const file = plant(checkout, '2026-09-20-Quokka-cache-warmup-before-deploy.md', remnant('https://github.com/acme/app/pull/77')); + + await publishQueuedLearnings(config, 'alice'); + + const published = (await publishedFiles(origin)).filter((f) => f.endsWith('.md')); + expect(published).toEqual(['learnings/quokka-copy-2026-09-21-bbb222.md']); + expect(fs.existsSync(file)).toBe(false); + }); + + it('is left where it is when init switched the install after this command loaded its config, even though the new queue has the same merge request', async () => { + const { config, origin, checkout } = await setUp(); + const file = plant(checkout, '2026-09-20-Quokka-cache-warmup-before-deploy.md', remnant()); + // What init to another team repo leaves: its config, and a learning its install queued. + const other: LocalConfig = { ...config, repo: { ...config.repo, remote: path.join(tmp, 'other.git') } }; + writeInstallConfig(other); + await savePendingLearning(other, 'quokka-2026-09-21-ccc444.md', remnant('https://github.com/acme/app/pull/42', 'Quokka, the other team')); + + const report = await publishQueuedLearnings(config, 'alice'); + + expect(report.installChanged).toBeDefined(); + expect(fs.readFileSync(file, 'utf8')).toBe(remnant()); + expect(await listPendingLearnings(other)).toEqual(['quokka-2026-09-21-ccc444.md']); + expect((await publishedFiles(origin)).filter((f) => f.endsWith('.md'))).toEqual([]); + }); + + it('is left where it is while another command holds the sync lock, which would queue it twice', async () => { + const { config, origin, checkout } = await setUp(); + const file = plant(checkout, '2026-09-20-Quokka-cache-warmup-before-deploy.md', remnant()); + const syncLock = path.join(getDataHome(config), SYNC_LOCK_FILENAME); + fs.writeFileSync(syncLock, JSON.stringify({ pid: process.pid, startedAt: new Date().toISOString(), owner: 'a pull' })); + + await publishQueuedLearnings(config, 'alice'); + + expect(fs.existsSync(file)).toBe(true); + expect(await listPendingLearnings(config)).toEqual([]); + expect((await publishedFiles(origin)).filter((f) => QUEUED_NAME.test(f))).toEqual([]); + }); + + it('is left where it is by a dry run', async () => { + const { config, origin, checkout } = await setUp(); + const file = plant(checkout, '2026-09-20-Quokka-cache-warmup-before-deploy.md', remnant()); + + await publishQueuedLearnings(config, 'alice', { dryRun: true }); + + expect(fs.existsSync(file)).toBe(true); + expect((await publishedFiles(origin)).filter((f) => QUEUED_NAME.test(f))).toEqual([]); + expect(await listPendingLearnings(config)).toEqual([]); + }); +}); + +describe('a dry run (#823 item 20)', () => { + it('publishes nothing from the queue and reports it as still queued', async () => { + const { config, origin } = await setUp(); + await savePendingLearning(config, 'otter-2026-09-25-ccc333.md', '---\ntitle: Otter\n---\nOtters hold hands.\n'); + + const report = await publishQueuedLearnings(config, 'alice', { dryRun: true }); + + expect(report).toEqual({ published: [], remaining: 1 }); + expect((await publishedFiles(origin)).filter((f) => f.endsWith('.md'))).toEqual([]); + expect(await listPendingLearnings(config)).toEqual(['otter-2026-09-25-ccc333.md']); + }); +}); + +describe('publishing what maintenance changed (#823)', () => { + it('publishes the tracked change, and neither fails on nor publishes a removed file git never tracked', async () => { + const { config, origin, checkout } = await setUp(); + await savePendingLearning(config, 'kept-2026-01-01-aaa111.md', '---\ntitle: Kept\nconfidence: 0.5\n---\nKept.\n'); + await publishQueuedLearnings(config, 'alice'); + // Maintenance rewrote a tracked learning and pruned one nobody had committed. + const rewritten = path.join(checkout, 'learnings', 'kept-2026-01-01-aaa111.md'); + fs.writeFileSync(rewritten, '---\ntitle: Kept\nconfidence: 0.9\n---\nKept.\n'); + const prunedUntracked = path.join(checkout, 'learnings', 'draft.md'); + + const result = await publishLearningsMaintenance(config, '[teamai] Maintenance', [rewritten, prunedUntracked]); + + expect(result).toEqual({ status: 'published' }); + expect(await publishedContent(origin, 'learnings/kept-2026-01-01-aaa111.md')).toContain('confidence: 0.9'); + }); + + it('stages a filename with [ or * as itself, not as a pattern that sweeps in stray files', async () => { + const { config, origin, checkout } = await setUp(); + await savePendingLearning(config, 'a[1].md', '---\ntitle: A\n---\nA.\n'); + await publishQueuedLearnings(config, 'alice'); + // Maintenance removed a tracked `a[1].md` and wrote a new `b*.md`. The strays + // match those names as glob patterns and nobody committed them. + const removed = path.join(checkout, 'learnings', 'a[1].md'); + fs.rmSync(removed); + const written = path.join(checkout, 'learnings', 'b*.md'); + fs.writeFileSync(written, '---\ntitle: B\n---\nB.\n'); + fs.writeFileSync(path.join(checkout, 'learnings', 'a1.md'), 'stray\n'); + fs.writeFileSync(path.join(checkout, 'learnings', 'bee.md'), 'stray\n'); + + const result = await publishLearningsMaintenance(config, '[teamai] Maintenance', [removed, written]); + + expect(result).toEqual({ status: 'published' }); + expect((await publishedFiles(origin)).filter((f) => f.endsWith('.md')).sort()).toEqual(['learnings/b*.md']); + }); + + it('commits only what maintenance changed, leaving a file someone else staged in the checkout staged', async () => { + const { config, origin, checkout } = await setUp(); + await savePendingLearning(config, 'kept-2026-01-01-aaa111.md', '---\ntitle: Kept\nconfidence: 0.5\n---\nKept.\n'); + await publishQueuedLearnings(config, 'alice'); + plant(checkout, 'staged-by-hand.md', 'not maintenance\n'); + await simpleGit(checkout).add(['learnings/staged-by-hand.md']); + const rewritten = path.join(checkout, 'learnings', 'kept-2026-01-01-aaa111.md'); + fs.writeFileSync(rewritten, '---\ntitle: Kept\nconfidence: 0.9\n---\nKept.\n'); + + const result = await publishLearningsMaintenance(config, '[teamai] Maintenance', [rewritten]); + + expect(result).toEqual({ status: 'published' }); + expect(await publishedFiles(origin)).not.toContain('learnings/staged-by-hand.md'); + expect(await simpleGit(checkout).raw(['diff', '--cached', '--name-only'])).toBe('learnings/staged-by-hand.md\n'); + }); + + it('publishes after a teammate pushed first, leaving a file someone else staged in the checkout staged', async () => { + const { config, origin, checkout } = await setUp(); + await savePendingLearning(config, 'kept-2026-01-01-aaa111.md', '---\ntitle: Kept\nconfidence: 0.5\n---\nKept.\n'); + await publishQueuedLearnings(config, 'alice'); + plant(checkout, 'staged-by-hand.md', 'not maintenance\n'); + await simpleGit(checkout).add(['learnings/staged-by-hand.md']); + const rewritten = path.join(checkout, 'learnings', 'kept-2026-01-01-aaa111.md'); + fs.writeFileSync(rewritten, '---\ntitle: Kept\nconfidence: 0.9\n---\nKept.\n'); + // The push is non-fast-forward, so the publish rebases, which git refuses with anything staged. + await teammatePublishes(origin, 'bob-2026-01-02-bbb222.md', '---\ntitle: Bob\n---\nBob.\n'); + + const result = await publishLearningsMaintenance(config, '[teamai] Maintenance', [rewritten]); + + expect(result).toEqual({ status: 'published' }); + expect(await publishedContent(origin, 'learnings/kept-2026-01-01-aaa111.md')).toContain('confidence: 0.9'); + expect(await publishedFiles(origin)).toContain('learnings/bob-2026-01-02-bbb222.md'); + expect(await publishedFiles(origin)).not.toContain('learnings/staged-by-hand.md'); + expect(await simpleGit(checkout).raw(['diff', '--cached', '--name-only'])).toBe('learnings/staged-by-hand.md\n'); + expect(fs.readFileSync(path.join(checkout, 'learnings', 'staged-by-hand.md'), 'utf8')).toBe('not maintenance\n'); + }); + + it('refreshes an unpushed commit onto a teammate\'s push, leaving a file someone else staged in the checkout staged', async () => { + const { config, origin, checkout } = await setUp(); + await savePendingLearning(config, 'kept-2026-01-01-aaa111.md', '---\ntitle: Kept\nconfidence: 0.5\n---\nKept.\n'); + await publishQueuedLearnings(config, 'alice'); + const rewritten = path.join(checkout, 'learnings', 'kept-2026-01-01-aaa111.md'); + fs.writeFileSync(rewritten, '---\ntitle: Kept\nconfidence: 0.9\n---\nKept.\n'); + expect((await offline(origin, () => publishLearningsMaintenance(config, '[teamai] Maintenance', [rewritten]))).status).toBe('failed'); + // An edit to a tracked file, which a plain `stash apply` restores unstaged (a new file it stages again). + fs.writeFileSync(path.join(checkout, '.gitignore'), 'edited by hand\n'); + await simpleGit(checkout).add(['.gitignore']); + await teammatePublishes(origin, 'bob-2026-01-02-bbb222.md', '---\ntitle: Bob\n---\nBob.\n'); + + await learningsBranch.refresh(config, { pushIfCreated: false }); + + expect(fs.existsSync(path.join(checkout, 'learnings', 'bob-2026-01-02-bbb222.md'))).toBe(true); + expect(await simpleGit(checkout).raw(['rev-list', '--count', 'origin/teamai-learnings..HEAD'])).toBe('1\n'); + expect(await simpleGit(checkout).raw(['diff', '--cached', '--name-only'])).toBe('.gitignore\n'); + }); + + it('keeps what someone else staged when a teammate changed the same lines and the publish has to rebase: an untouched file stays staged as it was, the conflicting one keeps its content unstaged and is named', async () => { + const { config, origin, checkout } = await setUp(); + await savePendingLearning(config, 'kept-2026-01-01-aaa111.md', '---\ntitle: Kept\nconfidence: 0.5\n---\nKept.\n'); + await savePendingLearning(config, 'shared-2026-01-01-ccc333.md', 'one\ntwo\nthree\n'); + await publishQueuedLearnings(config, 'alice'); + const shared = path.join(checkout, 'learnings', 'shared-2026-01-01-ccc333.md'); + fs.writeFileSync(shared, 'one\ntwo by hand\nthree\n'); + await simpleGit(checkout).add(['learnings/shared-2026-01-01-ccc333.md']); + // Staged in part, and a file the teammate did not touch: it keeps exactly that staging. + const gitignore = path.join(checkout, '.gitignore'); + const stagedIgnore = `${fs.readFileSync(gitignore, 'utf8')}staged-by-hand\n`; + fs.writeFileSync(gitignore, stagedIgnore); + await simpleGit(checkout).add(['.gitignore']); + fs.writeFileSync(gitignore, `${stagedIgnore}not staged\n`); + const rewritten = path.join(checkout, 'learnings', 'kept-2026-01-01-aaa111.md'); + fs.writeFileSync(rewritten, '---\ntitle: Kept\nconfidence: 0.9\n---\nKept.\n'); + // The staged edit no longer applies on top of the teammate's, so `stash apply --index` refuses. + await teammatePublishes(origin, 'shared-2026-01-01-ccc333.md', 'one\ntwo by bob\nthree\n'); + const warned = watchWarnings(); + + const result = await publishLearningsMaintenance(config, '[teamai] Maintenance', [rewritten]); + + expect(result).toEqual({ status: 'published' }); + expect(await publishedContent(origin, 'learnings/shared-2026-01-01-ccc333.md')).toBe('one\ntwo by bob\nthree\n'); + expect(fs.readFileSync(shared, 'utf8')).toBe('one\ntwo by hand\nthree\n'); + // Staging it again would stage a revert of the teammate's line. + expect(await simpleGit(checkout).raw(['diff', '--cached', '--name-only'])).toBe('.gitignore\n'); + expect(await simpleGit(checkout).raw(['show', ':.gitignore'])).toBe(stagedIgnore); + expect(fs.readFileSync(gitignore, 'utf8')).toBe(`${stagedIgnore}not staged\n`); + expect(warned()).toContain('learnings/shared-2026-01-01-ccc333.md'); + expect(warned()).not.toContain('.gitignore'); + }); + + it('keeps every other uncommitted file when a removal someone else staged conflicts with a teammate\'s edit of that file', async () => { + const { config, origin, checkout } = await setUp(); + await savePendingLearning(config, 'kept-2026-01-01-aaa111.md', '---\ntitle: Kept\nconfidence: 0.5\n---\nKept.\n'); + await savePendingLearning(config, 'shared-2026-01-01-ccc333.md', 'one\ntwo\nthree\n'); + await publishQueuedLearnings(config, 'alice'); + await simpleGit(checkout).rm(['learnings/shared-2026-01-01-ccc333.md']); + const gitignore = path.join(checkout, '.gitignore'); + const editedIgnore = `${fs.readFileSync(gitignore, 'utf8')}not staged\n`; + fs.writeFileSync(gitignore, editedIgnore); + const rewritten = path.join(checkout, 'learnings', 'kept-2026-01-01-aaa111.md'); + fs.writeFileSync(rewritten, '---\ntitle: Kept\nconfidence: 0.9\n---\nKept.\n'); + // Removed here, edited there: the plain apply leaves a modify/delete conflict. + await teammatePublishes(origin, 'shared-2026-01-01-ccc333.md', 'one\ntwo by bob\nthree\n'); + const warned = watchWarnings(); + + const result = await publishLearningsMaintenance(config, '[teamai] Maintenance', [rewritten]); + + expect(result).toEqual({ status: 'published' }); + expect(fs.readFileSync(gitignore, 'utf8')).toBe(editedIgnore); + expect(fs.existsSync(path.join(checkout, 'learnings', 'shared-2026-01-01-ccc333.md'))).toBe(false); + expect(await simpleGit(checkout).raw(['show', 'HEAD:learnings/shared-2026-01-01-ccc333.md'])).toBe('one\ntwo by bob\nthree\n'); + expect(warned()).toContain('learnings/shared-2026-01-01-ccc333.md'); + }); + + it('warns, naming it, when a file someone else staged merged with a teammate\'s change and can no longer be staged as it was', async () => { + const { config, origin, checkout } = await setUp(); + await savePendingLearning(config, 'kept-2026-01-01-aaa111.md', '---\ntitle: Kept\nconfidence: 0.5\n---\nKept.\n'); + await savePendingLearning(config, 'shared-2026-01-01-ccc333.md', 'one\ntwo\nthree\nfour\nfive\n'); + await publishQueuedLearnings(config, 'alice'); + const shared = path.join(checkout, 'learnings', 'shared-2026-01-01-ccc333.md'); + fs.writeFileSync(shared, 'one\ntwo by hand\nthree\nfour\nfive\n'); + await simpleGit(checkout).add(['learnings/shared-2026-01-01-ccc333.md']); + const rewritten = path.join(checkout, 'learnings', 'kept-2026-01-01-aaa111.md'); + fs.writeFileSync(rewritten, '---\ntitle: Kept\nconfidence: 0.9\n---\nKept.\n'); + // Inside the staged patch's context, so it no longer applies, and one line apart, so the files still merge. + await teammatePublishes(origin, 'shared-2026-01-01-ccc333.md', 'one\ntwo\nthree\nfour by bob\nfive\n'); + const warned = watchWarnings(); + + const result = await publishLearningsMaintenance(config, '[teamai] Maintenance', [rewritten]); + + expect(result).toEqual({ status: 'published' }); + expect(fs.readFileSync(shared, 'utf8')).toBe('one\ntwo by hand\nthree\nfour by bob\nfive\n'); + expect(await simpleGit(checkout).raw(['diff', '--cached', '--name-only'])).toBe(''); + expect(warned()).toContain('learnings/shared-2026-01-01-ccc333.md'); + }); + + it('commits nothing when maintenance has nothing to stage, even with a file someone else staged in the checkout', async () => { + const { config, origin, checkout } = await setUp(); + plant(checkout, 'staged-by-hand.md', 'not maintenance\n'); + await simpleGit(checkout).add(['learnings/staged-by-hand.md']); + const prunedUntracked = path.join(checkout, 'learnings', 'draft.md'); + + const result = await publishLearningsMaintenance(config, '[teamai] Maintenance', [prunedUntracked]); + + expect(result).toEqual({ status: 'already-present' }); + expect(await publishedFiles(origin)).not.toContain('learnings/staged-by-hand.md'); + expect(await simpleGit(checkout).raw(['diff', '--cached', '--name-only'])).toBe('learnings/staged-by-hand.md\n'); + }); + + it('publishes a change another write kept it from publishing on the next publish, with nothing queued', async () => { + const { config, origin, checkout } = await setUp(); + await savePendingLearning(config, 'kept-2026-01-01-aaa111.md', '---\ntitle: Kept\nconfidence: 0.5\n---\nKept.\n'); + await publishQueuedLearnings(config, 'alice'); + const rewritten = path.join(checkout, 'learnings', 'kept-2026-01-01-aaa111.md'); + fs.writeFileSync(rewritten, '---\ntitle: Kept\nconfidence: 0.9\n---\nKept.\n'); + const release = holdLearningsLock(config); + const busy = await publishLearningsMaintenance(config, '[teamai] Maintenance', [rewritten]); + release(); + expect(busy).toEqual({ status: 'busy' }); + + await publishQueuedLearnings(config, 'alice'); + + expect(await publishedContent(origin, 'learnings/kept-2026-01-01-aaa111.md')).toContain('confidence: 0.9'); + expect(await simpleGit(checkout).raw(['status', '--porcelain'])).toBe(''); + }); + + it('does not publish, as that maintenance run, an edit made by hand after another write kept it from publishing', async () => { + const { config, origin, checkout } = await setUp(); + await savePendingLearning(config, 'kept-2026-01-01-aaa111.md', '---\ntitle: Kept\nconfidence: 0.5\n---\nKept.\n'); + await savePendingLearning(config, 'other-2026-01-01-bbb222.md', '---\ntitle: Other\nconfidence: 0.5\n---\nOther.\n'); + await publishQueuedLearnings(config, 'alice'); + const rewritten = path.join(checkout, 'learnings', 'kept-2026-01-01-aaa111.md'); + const other = path.join(checkout, 'learnings', 'other-2026-01-01-bbb222.md'); + fs.writeFileSync(rewritten, '---\ntitle: Kept\nconfidence: 0.9\n---\nKept.\n'); + fs.writeFileSync(other, '---\ntitle: Other\nconfidence: 0.9\n---\nOther.\n'); + const release = holdLearningsLock(config); + expect(await publishLearningsMaintenance(config, '[teamai] Maintenance', [rewritten, other])).toEqual({ status: 'busy' }); + release(); + fs.writeFileSync(rewritten, '---\ntitle: Kept\nconfidence: 0.9\n---\nEdited by hand.\n'); + const warned = watchWarnings(); + + await publishQueuedLearnings(config, 'alice'); + + expect(await publishedContent(origin, 'learnings/kept-2026-01-01-aaa111.md')).not.toContain('Edited by hand'); + expect(await publishedContent(origin, 'learnings/other-2026-01-01-bbb222.md')).toContain('confidence: 0.9'); + expect(fs.readFileSync(rewritten, 'utf8')).toContain('Edited by hand'); + expect(warned()).toContain(path.join('learnings', 'kept-2026-01-01-aaa111.md')); + const records = path.resolve(checkout, (await simpleGit(checkout).raw(['rev-parse', '--git-path', 'teamai-maintenance'])).trim()); + expect(fs.readdirSync(records)).toEqual([]); + }); + + it('pushes a committed maintenance change a refresh merged with a teammate\'s edit, without calling that a hand edit', async () => { + const { config, origin, checkout } = await setUp(); + await savePendingLearning(config, 'kept-2026-01-01-aaa111.md', '---\ntitle: Kept\nconfidence: 0.5\n---\nKept.\n'); + await publishQueuedLearnings(config, 'alice'); + const rewritten = path.join(checkout, 'learnings', 'kept-2026-01-01-aaa111.md'); + fs.writeFileSync(rewritten, '---\ntitle: Kept\nconfidence: 0.9\n---\nKept.\n'); + expect((await offline(origin, () => publishLearningsMaintenance(config, '[teamai] Maintenance', [rewritten]))).status).toBe('failed'); + await teammatePublishes(origin, 'kept-2026-01-01-aaa111.md', '---\ntitle: Kept\nconfidence: 0.5\n---\nKept, said bob.\n'); + await learningsBranch.refresh(config, { pushIfCreated: false }); + const warned = watchWarnings(); + + await publishQueuedLearnings(config, 'alice'); + + expect(await publishedContent(origin, 'learnings/kept-2026-01-01-aaa111.md')).toBe('---\ntitle: Kept\nconfidence: 0.9\n---\nKept, said bob.\n'); + expect(warned()).toBe(''); + }); + + it('publishes an edit a newer maintenance run recorded over an older record of the same file', async () => { + const { config, origin, checkout } = await setUp(); + await savePendingLearning(config, 'kept-2026-01-01-aaa111.md', '---\ntitle: Kept\nconfidence: 0.5\n---\nKept.\n'); + await publishQueuedLearnings(config, 'alice'); + const rewritten = path.join(checkout, 'learnings', 'kept-2026-01-01-aaa111.md'); + const release = holdLearningsLock(config); + fs.writeFileSync(rewritten, '---\ntitle: Kept\nconfidence: 0.9\n---\nKept.\n'); + expect(await publishLearningsMaintenance(config, '[teamai] Confidence', [rewritten])).toEqual({ status: 'busy' }); + fs.writeFileSync(rewritten, '---\ntitle: Kept\nconfidence: 0.95\n---\nKept.\n'); + expect(await publishLearningsMaintenance(config, '[teamai] Confidence', [rewritten])).toEqual({ status: 'busy' }); + release(); + const warned = watchWarnings(); + + await publishQueuedLearnings(config, 'alice'); + + expect(await publishedContent(origin, 'learnings/kept-2026-01-01-aaa111.md')).toContain('confidence: 0.95'); + expect(warned()).toBe(''); + }); + + it('publishes a record written before records held content hashes, as before', async () => { + const { config, origin, checkout } = await setUp(); + await savePendingLearning(config, 'kept-2026-01-01-aaa111.md', '---\ntitle: Kept\nconfidence: 0.5\n---\nKept.\n'); + await publishQueuedLearnings(config, 'alice'); + fs.writeFileSync(path.join(checkout, 'learnings', 'kept-2026-01-01-aaa111.md'), '---\ntitle: Kept\nconfidence: 0.9\n---\nKept.\n'); + const records = path.resolve(checkout, (await simpleGit(checkout).raw(['rev-parse', '--git-path', 'teamai-maintenance'])).trim()); + fs.mkdirSync(records, { recursive: true }); + fs.writeFileSync(path.join(records, '0-legacy.json'), JSON.stringify({ message: '[teamai] Maintenance', files: [path.join('learnings', 'kept-2026-01-01-aaa111.md')] })); + + await publishQueuedLearnings(config, 'alice'); + + expect(await publishedContent(origin, 'learnings/kept-2026-01-01-aaa111.md')).toContain('confidence: 0.9'); + expect(fs.readdirSync(records)).toEqual([]); + }); + + it('publishes the readable records and removes, naming it, one a crash left truncated', async () => { + const { config, origin, checkout } = await setUp(); + await savePendingLearning(config, 'kept-2026-01-01-aaa111.md', '---\ntitle: Kept\nconfidence: 0.5\n---\nKept.\n'); + await publishQueuedLearnings(config, 'alice'); + const rewritten = path.join(checkout, 'learnings', 'kept-2026-01-01-aaa111.md'); + fs.writeFileSync(rewritten, '---\ntitle: Kept\nconfidence: 0.9\n---\nKept.\n'); + const release = holdLearningsLock(config); + expect(await publishLearningsMaintenance(config, '[teamai] Maintenance', [rewritten])).toEqual({ status: 'busy' }); + release(); + const records = path.resolve(checkout, (await simpleGit(checkout).raw(['rev-parse', '--git-path', 'teamai-maintenance'])).trim()); + const truncated = path.join(records, '0-truncated.json'); + fs.writeFileSync(truncated, '{"message":"[teamai] Prune","fi'); + const warned = watchWarnings(); + + await publishQueuedLearnings(config, 'alice'); + + expect(await publishedContent(origin, 'learnings/kept-2026-01-01-aaa111.md')).toContain('confidence: 0.9'); + expect(fs.readdirSync(records)).toEqual([]); + expect(warned()).toContain(truncated); + }); + + it('keeps, and warns about, a record it cannot read, while the readable records publish', async () => { + const { config, origin, checkout } = await setUp(); + await savePendingLearning(config, 'kept-2026-01-01-aaa111.md', '---\ntitle: Kept\nconfidence: 0.5\n---\nKept.\n'); + await publishQueuedLearnings(config, 'alice'); + const rewritten = path.join(checkout, 'learnings', 'kept-2026-01-01-aaa111.md'); + fs.writeFileSync(rewritten, '---\ntitle: Kept\nconfidence: 0.9\n---\nKept.\n'); + const release = holdLearningsLock(config); + expect(await publishLearningsMaintenance(config, '[teamai] Maintenance', [rewritten])).toEqual({ status: 'busy' }); + release(); + const records = path.resolve(checkout, (await simpleGit(checkout).raw(['rev-parse', '--git-path', 'teamai-maintenance'])).trim()); + // A read that fails, as on a permission or I/O error, says nothing about the record itself. + const unreadable = path.join(records, '0-unreadable.json'); + fs.writeFileSync(unreadable, JSON.stringify({ message: '[teamai] Prune', files: [] })); + const readFile = fs.promises.readFile; + vi.spyOn(fs.promises, 'readFile').mockImplementation(async (file, options) => { + if (file === unreadable) throw Object.assign(new Error(`EACCES: permission denied, open '${unreadable}'`), { code: 'EACCES' }); + return readFile(file, options); + }); + const warned = watchWarnings(); + + await publishQueuedLearnings(config, 'alice'); + + expect(await publishedContent(origin, 'learnings/kept-2026-01-01-aaa111.md')).toContain('confidence: 0.9'); + expect(fs.readdirSync(records)).toEqual(['0-unreadable.json']); + expect(warned()).toContain(unreadable); + expect(warned()).not.toContain('Removed'); + }); + + it('pushes a change it committed and could not push on the next publish, with nothing queued', async () => { + const { config, origin, checkout } = await setUp(); + await savePendingLearning(config, 'stale-2026-01-01-aaa111.md', '---\ntitle: Stale\n---\nStale.\n'); + await publishQueuedLearnings(config, 'alice'); + const pruned = path.join(checkout, 'learnings', 'stale-2026-01-01-aaa111.md'); + fs.rmSync(pruned); + const failed = await offline(origin, () => publishLearningsMaintenance(config, '[teamai] Prune 1 learning(s)', [pruned])); + expect(failed.status).toBe('failed'); + + await publishQueuedLearnings(config, 'alice'); + + expect(await publishedFiles(origin)).not.toContain('learnings/stale-2026-01-01-aaa111.md'); + }); +}); + +describe('publishing what maintenance changed for an HTTP install (#823)', () => { + it('reports a failed publish, not a throw, after a prune removed a file from the HTTP cache', async () => { + const cache = path.join(tmp, 'http-cache'); + const removed = path.join(cache, 'learnings', 'stale-2026-01-01-aaa111.md'); + fs.mkdirSync(path.dirname(removed), { recursive: true }); + const config: LocalConfig = { + repo: { localPath: cache, remote: 'https://team.example/api', kind: 'http', url: 'https://team.example/api' }, + username: 'alice', + scope: 'user', + additionalRoles: [], + }; + writeInstallConfig(config); + + const result = await publishLearningsMaintenance(config, '[teamai] Maintenance', [removed]); + + expect(result.status).toBe('failed'); + }); +}); diff --git a/src/__tests__/maintenance-promote.test.ts b/src/__tests__/maintenance-promote.test.ts index 04a0ff15b..2328c4daa 100644 --- a/src/__tests__/maintenance-promote.test.ts +++ b/src/__tests__/maintenance-promote.test.ts @@ -133,8 +133,9 @@ describe('executePromotion', () => { suggestedCategory: 'skills' as const, }; - const targetPath = await executePromotion(candidate, tmpDir, { dryRun: true }); + const { targetPath, marked } = await executePromotion(candidate, tmpDir, { dryRun: true }); expect(targetPath).toContain('skills/candidate.md'); + expect(marked).toBeNull(); expect(fs.existsSync(path.join(tmpDir, 'skills', 'candidate.md'))).toBe(false); }); @@ -155,7 +156,8 @@ describe('executePromotion', () => { suggestedCategory: 'skills' as const, }; - await executePromotion(candidate, tmpDir, { category: 'skills' }); + const { marked } = await executePromotion(candidate, tmpDir, { category: 'skills' }); + expect(marked).toBe(path.join(learningsDir, 'ready.md')); expect(fs.existsSync(path.join(skillsDir, 'ready.md'))).toBe(true); diff --git a/src/__tests__/maintenance-two-roots.test.ts b/src/__tests__/maintenance-two-roots.test.ts index 25a8c0dce..3ee8286db 100644 --- a/src/__tests__/maintenance-two-roots.test.ts +++ b/src/__tests__/maintenance-two-roots.test.ts @@ -39,13 +39,13 @@ describe('maintenance across the write root and the inherited root', () => { it('writes an inherited learning back into the write root, leaving the clone untouched', async () => { fs.writeFileSync(path.join(inherited, 'old.md'), '---\ntitle: old\nconfidence: 0.1\n---\nbody'); - const updated = await writeBackConfidence( + const written = await writeBackConfidence( [writeRoot, inherited], new Map([['old', 0.9]]), writeRoot, ); - expect(updated).toBe(1); + expect(written).toEqual([path.join(writeRoot, 'old.md')]); expect(fs.readFileSync(path.join(writeRoot, 'old.md'), 'utf8')).toContain('confidence: 0.9'); expect(fs.readFileSync(path.join(inherited, 'old.md'), 'utf8')).toContain('confidence: 0.1'); }); @@ -69,7 +69,7 @@ describe('maintenance across the write root and the inherited root', () => { reason: 'test', }]); - expect(result).toEqual({ archived: 0, removed: 0 }); + expect(result).toEqual({ archived: 0, removed: 0, changed: [] }); expect(fs.existsSync(path.join(inherited, 'old.md'))).toBe(true); expect(vi.mocked(log.warn).mock.calls.join(' ')).toContain('pull request'); }); @@ -86,6 +86,7 @@ describe('maintenance across the write root and the inherited root', () => { }], { archive: true }); expect(result.archived).toBe(1); + expect(result.changed).toEqual([path.join(writeRoot, 'stale.md'), path.join(writeRoot, '_archive', 'stale.md')]); expect(fs.existsSync(path.join(writeRoot, '_archive', 'stale.md'))).toBe(true); expect(fs.existsSync(path.join(writeRoot, 'stale.md'))).toBe(false); }); diff --git a/src/__tests__/migrate.test.ts b/src/__tests__/migrate.test.ts index 8fb2c0048..307e06e5e 100644 --- a/src/__tests__/migrate.test.ts +++ b/src/__tests__/migrate.test.ts @@ -389,6 +389,22 @@ describe('runMigration', () => { expect(await planMigration(repoRoot)).toBeNull(); }); + it('says where the data already lives when it retires a leftover, without claiming a migration was interrupted (#823)', async () => { + // A linked worktree whose checkout never ran teamai lands here too: another + // checkout built the partition, nothing was interrupted. + await seedLegacyLayout(); + const partition = projectDataHome(repoRoot); + await writePartitionConfig(partition); + vi.mocked(log.success).mockClear(); + + const plan = await planMigration(repoRoot); + if (plan?.mode !== 'retire-only') throw new Error('expected a retire-only plan'); + expect(await runMigration(plan)).toBe('migrated'); + + const said = vi.mocked(log.success).mock.calls.map((c) => String(c[0])); + expect(said).toEqual([`Retired ${legacyDir} to ${legacyDir}.bak: this project's data already lives in ${partition}`]); + }); + it('keeps the legacy dir when an unreadable partition config appears after planning (#797)', async () => { // The re-check under the lock must use the same notion of "built" as the plan. await seedLegacyLayout(); diff --git a/src/__tests__/reports-branch-readonly.test.ts b/src/__tests__/reports-branch-readonly.test.ts index 6ce850180..37103c533 100644 --- a/src/__tests__/reports-branch-readonly.test.ts +++ b/src/__tests__/reports-branch-readonly.test.ts @@ -142,6 +142,16 @@ describe('ensureReportsWorktree read-only cold start', () => { }); }); +/** + * `diff --cached --name-only -z -- <files>` lists `staged`, what the publish commits. + * Any other git call is `rev-list --count origin/<branch>..HEAD`, which the publish + * checks before reporting success: '0' is "nothing left to deliver". + */ +function stageInWorktree(staged: string[]): void { + mocks.worktreeGit.raw.mockImplementation(async (args: string[]) => + args.includes('diff') ? staged.map((f) => `${f}\0`).join('') : '0'); +} + describe('commitAndPushReports', () => { beforeEach(() => { vi.clearAllMocks(); @@ -150,10 +160,7 @@ describe('commitAndPushReports', () => { mocks.worktreeGit.commit.mockResolvedValue(undefined); mocks.worktreeGit.push.mockResolvedValue(undefined); mockCheckoutOfBusinessRepo(); - // `rev-list --count origin/<branch>..HEAD`: the publish confirms the ref - // moved before reporting success. '0' is "nothing left to deliver". - mocks.worktreeGit.raw.mockResolvedValue('0'); - mocks.worktreeGit.status.mockResolvedValue({ staged: ['members/alice.yaml'] }); + stageInWorktree(['members/alice.yaml']); vi.mocked(acquireLock).mockResolvedValue(true); vi.mocked(releaseLock).mockResolvedValue(undefined); }); @@ -169,7 +176,7 @@ describe('commitAndPushReports', () => { }); it('pushes an unchanged report retry without making an empty commit', async () => { - mocks.worktreeGit.status.mockResolvedValue({ staged: [] }); + stageInWorktree([]); expect(await commitAndPushReports(config, 'retry stats', ['stats/alice.yaml'], { pushIfUnchanged: true })).toBe(true); expect(mocks.worktreeGit.commit).not.toHaveBeenCalled(); expect(mocks.worktreeGit.push).toHaveBeenCalledWith(['origin', 'teamai-reports']); @@ -177,7 +184,7 @@ describe('commitAndPushReports', () => { }); it('preserves the default no-op when no changes are staged', async () => { - mocks.worktreeGit.status.mockResolvedValue({ staged: [] }); + stageInWorktree([]); expect(await commitAndPushReports(config, 'unchanged member', ['members/'])).toBe(false); expect(mocks.worktreeGit.push).not.toHaveBeenCalled(); }); @@ -260,9 +267,12 @@ describe('readableReportsWorktree', () => { expect(mocks.worktreeGit.raw).not.toHaveBeenCalled(); }); - it('refreshes and ensures the checkout, publishing nothing, when the lock is free', async () => { + it('refreshes the checkout, publishing nothing, when the lock is free', async () => { vi.mocked(acquireLock).mockResolvedValue(true); mocks.isGitRepo.mockResolvedValue(true); + // A clean checkout, level with origin: the refresh completes. + mocks.worktreeGit.status.mockResolvedValue({ staged: [], renamed: [], conflicted: [], isClean: () => true }); + mocks.worktreeGit.raw.mockResolvedValue('0'); await expect(readableReportsWorktree(config)).resolves.toBe(WT); diff --git a/src/__tests__/reports-reader-lock.test.ts b/src/__tests__/reports-reader-lock.test.ts new file mode 100644 index 000000000..6753fb30c --- /dev/null +++ b/src/__tests__/reports-reader-lock.test.ts @@ -0,0 +1,76 @@ +/** + * A reports reader whose refresh failed must not create the checkout without + * the reports lock (#823 item 15): a writer that took the lock meanwhile may be + * creating it at the same path (#808). + */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { simpleGit } from 'simple-git'; + +import { REPORTS_LOCK_FILENAME, REPORTS_WORKTREE_DIRNAME, type LocalConfig } from '../types.js'; + +// The first refresh fails before it creates anything, as a transient failure would. +vi.mock('../utils/git.js', async (importOriginal) => { + const actual = await importOriginal<typeof import('../utils/git.js')>(); + return { ...actual, isDedicatedRepoRoot: vi.fn(actual.isDedicatedRepoRoot).mockResolvedValueOnce(false) }; +}); + +// A writer takes the reports lock the moment the failed refresh lets it go. +const writer = vi.hoisted(() => ({ took: false })); +vi.mock('../update.js', async (importOriginal) => { + const actual = await importOriginal<typeof import('../update.js')>(); + return { + ...actual, + releaseLock: async (lockPath?: string) => { + await actual.releaseLock(lockPath); + if (!writer.took && lockPath && path.basename(lockPath) === REPORTS_LOCK_FILENAME) { + writer.took = true; + fs.writeFileSync(lockPath, JSON.stringify({ pid: process.pid, startedAt: new Date().toISOString(), owner: 'writer' })); + } + }, + }; +}); + +const { readableReportsWorktree } = await import('../utils/reports-branch.js'); + +let tmp: string; + +beforeEach(() => { + tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'teamai-reports-reader-lock-')); +}); + +afterEach(() => { + fs.rmSync(tmp, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); +}); + +async function gitInstall(): Promise<LocalConfig> { + const seed = path.join(tmp, 'seed'); + fs.mkdirSync(seed); + const seedGit = simpleGit(seed); + await seedGit.init(['--initial-branch=main']); + await seedGit.addConfig('user.email', 't@t.com'); + await seedGit.addConfig('user.name', 't'); + fs.writeFileSync(path.join(seed, 'teamai.yaml'), 'team: acme\n'); + await seedGit.add(['.']); + await seedGit.commit('init'); + const origin = path.join(tmp, 'origin.git'); + await simpleGit().clone(seed, origin, ['--bare']); + const clone = path.join(tmp, 'team-repo'); + await simpleGit().clone(origin, clone); + return { repo: { localPath: clone, remote: origin, kind: 'git' }, username: 'alice', scope: 'user', additionalRoles: [] }; +} + +describe('readableReportsWorktree after a failed refresh (#823 item 15)', () => { + it('creates no checkout while a writer holds the reports lock', async () => { + const config = await gitInstall(); + const checkout = path.join(tmp, REPORTS_WORKTREE_DIRNAME); + + const dir = await readableReportsWorktree(config); + + expect(writer.took).toBe(true); + expect(dir).toBe(checkout); + expect(fs.existsSync(checkout)).toBe(false); + }); +}); diff --git a/src/__tests__/single-repo-mode.test.ts b/src/__tests__/single-repo-mode.test.ts index 186f9f5b2..071919b80 100644 --- a/src/__tests__/single-repo-mode.test.ts +++ b/src/__tests__/single-repo-mode.test.ts @@ -14,7 +14,7 @@ import { TeamaiConfigSchema, type LocalConfig, } from '../types.js'; -import { buildSelfModeGitignore, migrateSelfModeGitignore, migrateSelfModeGitignoreContent } from '../init.js'; +import { buildProjectScopeGitignore, buildSelfModeGitignore, migrateSelfModeGitignore, migrateSelfModeGitignoreContent } from '../init.js'; /** The names of `names` that `gitignore`, as `.teamai/.gitignore`, makes git ignore. */ function ignoredBy(gitignore: string, names: string[]): string[] { @@ -38,6 +38,8 @@ function ignoredBy(gitignore: string, names: string[]): string[] { /** The files a usage write leaves beside `usage.jsonl` while it runs or when it gives up on the lock (#788). */ const USAGE_SIDE_FILES = ['usage.jsonl.lock', 'usage.jsonl.123.0123456789ab.tmp', 'usage.pending-0f8e2c1a-1b2c-4d5e-8f90-123456789abc.jsonl']; +/** The temp copy an interrupted config save leaves beside config.yaml (writeFileAtomic, #831). */ +const CONFIG_SAVE_TEMP = 'config.yaml.123.0123456789ab.tmp'; function makeConfig(kind: 'git' | 'http' | 'self', localPath = '/repo/.teamai'): LocalConfig { return { @@ -172,6 +174,10 @@ describe('buildSelfModeGitignore', () => { expect(ignoredBy(gi, USAGE_SIDE_FILES)).toEqual(USAGE_SIDE_FILES); }); + it('ignores the temp copy an interrupted config save leaves (#823)', () => { + expect(ignoredBy(gi, [CONFIG_SAVE_TEMP])).toEqual([CONFIG_SAVE_TEMP]); + }); + it('ignores the learnings worktree, its lock and the queue, so contributing leaves git status clean', () => { const lines = gi.split('\n').map((l) => l.trim()); expect(lines).toContain('learnings-wt/'); @@ -180,6 +186,13 @@ describe('buildSelfModeGitignore', () => { }); }); +describe('buildProjectScopeGitignore', () => { + it('ignores the local config and the temp copy an interrupted config save leaves (#823)', () => { + const names = ['config.yaml', CONFIG_SAVE_TEMP, 'state.json', 'token']; + expect(ignoredBy(buildProjectScopeGitignore(), names)).toEqual(names); + }); +}); + describe('migrateSelfModeGitignore', () => { it('leaves the .gitignore whole when the disk fills while it is healed', async () => { const root = fs.mkdtempSync(path.join(os.tmpdir(), 'teamai-self-heal-')); @@ -221,7 +234,7 @@ describe('migrateSelfModeGitignoreContent (self-heal old gitignore)', () => { const old = [ 'env.sh', 'env.local', 'teamai.lock', 'learnings-wt/', '.learnings-lock', 'pending-learnings/', 'env/', - 'usage.jsonl.*', 'usage.pending-*.jsonl', + 'usage.jsonl.*', 'usage.pending-*.jsonl', 'config.yaml.*.tmp', ].join('\n'); const { changed, content } = migrateSelfModeGitignoreContent(old); expect(changed).toBe(false); // nothing to remove, env.local already present @@ -252,7 +265,7 @@ describe('migrateSelfModeGitignoreContent (self-heal old gitignore)', () => { const old = [ '# env is machine-local', 'config.yaml', 'env.local', 'teamai.lock', 'learnings-wt/', '.learnings-lock', 'pending-learnings/', - 'usage.jsonl.*', 'usage.pending-*.jsonl', + 'usage.jsonl.*', 'usage.pending-*.jsonl', 'config.yaml.*.tmp', ].join('\n'); const { changed, content } = migrateSelfModeGitignoreContent(old); // No bare `env` line, env.local already present → unchanged. @@ -275,6 +288,14 @@ describe('migrateSelfModeGitignoreContent (self-heal old gitignore)', () => { expect(content).toContain('usage.jsonl\nusage.jsonl.*\nusage.pending-*.jsonl\nknown-skills.json'); }); + it('adds the temp copy an interrupted config save leaves, after config.yaml (#823)', () => { + const old = ['config.yaml', 'state.json', 'token', 'teamai.lock', 'env.local'].join('\n'); + const { changed, content } = migrateSelfModeGitignoreContent(old); + expect(changed).toBe(true); + expect(content).toContain('config.yaml\nconfig.yaml.*.tmp\nstate.json'); + expect(ignoredBy(content, [CONFIG_SAVE_TEMP])).toEqual([CONFIG_SAVE_TEMP]); + }); + it('adds the learnings worktree, its lock and the queue', () => { const old = ['config.yaml', 'token', 'teamai.lock', 'env.local', 'reports-wt/', 'knowledge-wt/'].join('\n'); const { changed, content } = migrateSelfModeGitignoreContent(old); diff --git a/src/__tests__/votes-e2e.test.ts b/src/__tests__/votes-e2e.test.ts index c03521518..3da3afb15 100644 --- a/src/__tests__/votes-e2e.test.ts +++ b/src/__tests__/votes-e2e.test.ts @@ -224,8 +224,8 @@ describe('Phase 3+4 end-to-end data flow', () => { fs.writeFileSync(path.join(votesDir, 'user1.yaml'), YAML.stringify(v2)); const map = await computeAllConfidence(votesDir); - const updated = await writeBackConfidence([learningsDir], map); - expect(updated).toBe(1); + const written = await writeBackConfidence([learningsDir], map); + expect(written).toEqual([path.join(learningsDir, 'test-doc.md')]); // Verify frontmatter has confidence const afterContent = fs.readFileSync(path.join(learningsDir, 'test-doc.md'), 'utf-8'); diff --git a/src/contribute.ts b/src/contribute.ts index 0a14e9420..78209a3eb 100644 --- a/src/contribute.ts +++ b/src/contribute.ts @@ -5,13 +5,13 @@ import { assertNotReadOnly } from './read-only.js'; import { pathExists } from './utils/fs.js'; import { log, spinner } from './utils/logger.js'; import { markContributed } from './contribute-check.js'; -import { pendingLearningsDir, queueOwner, queueWriteRefusal, savePendingLearning } from './utils/pending-learnings.js'; +import { pendingLearningsDir, queueWriteRefusal, savePendingLearning } from './utils/pending-learnings.js'; import { publishQueuedLearnings } from './utils/learnings-publish.js'; import { indexableLearningsRoots } from './utils/learnings-roots.js'; import { resolveActiveLearningsNamespaces } from './projects.js'; import { isSafeNamespaceSegment } from './manifest-schema.js'; import type { GlobalOptions, LocalConfig } from './types.js'; -import { getBusinessRoot, getDataHome, getProjectSearchIndexPath, isSelfMode } from './types.js'; +import { getProjectSearchIndexPath, isSelfMode } from './types.js'; /** * Decide which learnings subdirectory a contribution lands in — resolved from @@ -102,31 +102,6 @@ export async function rebuildIndexAfterContribute(localConfig: LocalConfig): Pro // └─ done // -/** - * A queue an older teamai kept in this checkout's `.teamai/` is deleted with a - * linked worktree; move it into the shared queue, so the publish that follows - * sends it too (#808), or aside when the partition now serves another install - * (settleCheckoutQueue). The migration before contribute and import --from-mr - * does it too, and they stop when the queue would stay in the checkout - * (queueKeptInCheckout); this is a second pass. Best effort: the learning - * being queued must still be saved, and the old queue stays where it is. - */ -export async function drainCheckoutQueue(localConfig: LocalConfig): Promise<void> { - if (!isSelfMode(localConfig)) return; - const legacyDir = path.join(getBusinessRoot(localConfig), '.teamai'); - const dataHome = getDataHome(localConfig); - if (path.resolve(legacyDir) === path.resolve(dataHome)) return; - const { settleCheckoutQueue } = await import('./migrate.js'); - try { - await settleCheckoutQueue(legacyDir, dataHome, queueOwner(localConfig)); - } catch (e) { - log.warn( - `Could not move the learnings an older teamai queued in ${legacyDir} ` + - `(${e instanceof Error ? e.message : String(e)}). They stay there; the next teamai pull moves them.`, - ); - } -} - /** * Where a learning saved for an install that changed before it was published * is, for the member: still queued, or set aside with the previous install's @@ -230,7 +205,6 @@ export async function contribute( if (isSelfMode(localConfig)) { const { migrateSelfModeGitignore } = await import('./init.js'); await migrateSelfModeGitignore(localConfig); - await drainCheckoutQueue(localConfig); } try { diff --git a/src/import.ts b/src/import.ts index 8d6833e52..c96a0332a 100644 --- a/src/import.ts +++ b/src/import.ts @@ -246,7 +246,7 @@ export async function importCmd(opts: ImportOptions): Promise<void> { if (!opts.dryRun && !opts.output) assertNotReadOnly(localConfig, 'teamai import --from-mr'); // As contribute: into the active project's learnings namespace when there // is exactly one, else the shared root. - const { drainCheckoutQueue, resolveLearningsSubdir } = await import('./contribute.js'); + const { resolveLearningsSubdir } = await import('./contribute.js'); const learningsSubdir = opts.dryRun || opts.output ? '' : await resolveLearningsSubdir(localConfig); // The namespaces recall finds learnings in here (#823). The duplicate // check is advisory: a broken projects.yaml narrows it to the shared @@ -256,8 +256,6 @@ export async function importCmd(opts: ImportOptions): Promise<void> { log.warn(`The duplicate check reads the shared learnings only: ${e instanceof Error ? e.message : String(e)}`); return []; }); - // As contribute, before the extraction dedupes against the queue it writes into. - if (!opts.dryRun && !opts.output) await drainCheckoutQueue(localConfig); // Publishing creates a worktree under `.teamai/`; self-heal the ignore // rule first, as contribute does, while its notice can still be seen. if (!opts.dryRun && !opts.output) { diff --git a/src/index.ts b/src/index.ts index b82e5e4a5..5176d42fc 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1271,9 +1271,9 @@ recallCmd if (cmdOpts.confidenceWriteback) { const { computeAllConfidence, writeBackConfidence } = await import('./maintenance/index.js'); const map = await computeAllConfidence(votesDir); - const updated = await writeBackConfidence(learningsReadDirs, map, learningsWriteDir); - if (updated > 0) { - await publishMaintenance(localConfig, `[teamai] Update confidence for ${updated} learning(s)`); + const written = await writeBackConfidence(learningsReadDirs, map, learningsWriteDir); + if (written.length > 0) { + await publishMaintenance(localConfig, `[teamai] Update confidence for ${written.length} learning(s)`, written); } return; } @@ -1301,6 +1301,7 @@ recallCmd await publishMaintenance( localConfig, `[teamai] Prune ${pruned.archived + pruned.removed} learning(s)`, + pruned.changed, ); } return; @@ -1375,13 +1376,13 @@ recallCmd return; } - await executePromotion(candidate, repoPath, { + const { marked } = await executePromotion(candidate, repoPath, { category: cmdOpts.category as 'skills' | 'rules' | 'docs' | undefined, dryRun: cmdOpts.dryRun, learningsWriteDir, }); - if (!cmdOpts.dryRun) { - await publishMaintenance(localConfig, `[teamai] Mark ${candidate.docId} as promoted`); + if (marked) { + await publishMaintenance(localConfig, `[teamai] Mark ${candidate.docId} as promoted`, [marked]); } }); @@ -1412,14 +1413,14 @@ async function maintenancePathsOrExit(localConfig: LocalConfig): Promise<Mainten } /** - * Publish what a maintenance command just changed in the learnings worktree. - * Best-effort: the change is already on disk, so a failure to publish is worth + * Publish the files a maintenance command just changed in the learnings + * worktree, and nothing else there. Best-effort: the change is already on disk, so a failure to publish is worth * reporting but never worth failing the command over. */ -async function publishMaintenance(localConfig: LocalConfig, message: string): Promise<void> { +async function publishMaintenance(localConfig: LocalConfig, message: string, changed: readonly string[]): Promise<void> { const { publishLearningsMaintenance } = await import('./utils/learnings-publish.js'); const { log } = await import('./utils/logger.js'); - const result = await publishLearningsMaintenance(localConfig, message); + const result = await publishLearningsMaintenance(localConfig, message, changed); if (result.status === 'published') { log.success('Published maintenance changes to the learnings branch'); } else if (result.status === 'failed') { diff --git a/src/init.ts b/src/init.ts index ef6d6267a..174d151fa 100644 --- a/src/init.ts +++ b/src/init.ts @@ -36,6 +36,40 @@ async function settleModeSwitch(previous: LocalConfig | null, next: LocalConfig, process.exit(1); } } + +/** + * Move a config that names another install out of the way, to a free + * `config.yaml.previous` name beside it, before init clones the new team repo + * where that install's clone was. If init then stops before it saves the new + * config, no command runs the old config against the new clone (#823 item 17): + * they all ask for `teamai init` instead. Nothing is deleted. + */ +async function moveConfigAside(configPath: string, next: LocalConfig): Promise<void> { + let aside = `${configPath}.previous`; + for (let n = 1; await pathExists(aside); n++) aside = `${configPath}.previous.${n}`; + await fs.promises.rename(configPath, aside); + log.warn( + `Moved ${configPath} to ${aside}: it is another install's config, and init replaces its team repo with ` + + `${redactGitCredentials(next.repo.remote)}. If init stops before it finishes, run it again.`, + ); +} + +/** + * The config {@link moveConfigAside} set aside last. An init that stopped after + * it (a failed clone, an unknown --role) saved no config, so the rerun reads + * this one for the settings a re-init carries forward instead of dropping them. + */ +async function loadConfigSetAside(configPath: string): Promise<LocalConfig | null> { + let latest: string | undefined; + for (let n = 0, aside = `${configPath}.previous`; await pathExists(aside); aside = `${configPath}.previous.${++n}`) latest = aside; + if (!latest) return null; + try { + return LocalConfigSchema.parse(YAML.parse(await readFileSafe(latest) ?? '')); + } catch (e) { + log.debug(`Not carrying settings from ${latest}: ${(e as Error).message}`); + return null; + } +} import { log, spinner } from './utils/logger.js'; import { CLAUDE_TOOL_ID, @@ -48,6 +82,7 @@ import { REPORTS_BRANCH, type GlobalOptions, type LocalConfig, + LocalConfigSchema, ProviderNameSchema, type ProviderName, type Scope, @@ -715,6 +750,8 @@ export function buildSelfModeGitignore(): string { '# As of P2 this data lives in ~/.teamai/projects/<slug>/; these entries guard', '# pre-P2 installs (pre-migration) and any un-relocated path.', 'config.yaml', + // The temp copy an interrupted config save leaves (writeFileAtomic, #831). + 'config.yaml.*.tmp', 'state.json', 'token', 'teamai.lock', @@ -761,6 +798,32 @@ export function buildSelfModeGitignore(): string { ].join('\n'); } +/** The `.gitignore` project-scope init writes beside its local config, once. */ +export function buildProjectScopeGitignore(): string { + return [ + '# teamai local config (do not commit)', + 'config.yaml', + // The temp copy an interrupted config save leaves (writeFileAtomic, #831). + 'config.yaml.*.tmp', + 'state.json', + 'token', + 'teamai.lock', + '.update-lock', + 'env', + 'env.sh', + 'sessions/', + 'dashboard/', + 'usage.jsonl', + 'usage.jsonl.*', + 'usage.pending-*.jsonl', + 'known-skills.json', + 'learnings/', + 'search-index.json', + 'votes/', + '', + ].join('\n'); +} + /** * Migrate an existing single-repo `.teamai/.gitignore` written by an older teamai. * Early versions ignored `env`, which hid `.teamai/env/env.yaml` from push and @@ -806,6 +869,8 @@ export function migrateSelfModeGitignoreContent(content: string): { changed: boo // The usage lock, rewrite temps and pending events arrived with the usage cap (#788). ensure('usage.jsonl.*', 'usage.jsonl'); ensure('usage.pending-*.jsonl', 'usage.jsonl.*'); + // config.yaml has been saved atomically, through a temp copy, since #831. + ensure('config.yaml.*.tmp', 'config.yaml'); return { changed, content: filtered.join('\n') }; } @@ -1350,12 +1415,20 @@ export async function init(options: GlobalOptions & { return; } const existingLocalConfig = await loadLocalConfigForScope(scope, projectRoot); + const teamaiHome = scope === 'project' && projectRoot + ? (existingLocalConfig?.dataHome ?? await resolveProjectDataHome(projectRoot)) + : getTeamaiHome(scope, projectRoot); + const existingConfigPath = path.join(teamaiHome, 'config.yaml'); + // The settings this re-init carries forward: the live config's, or, when an + // init stopped after moving it aside, the one it set aside (#823 item 17). + const carriedConfig = existingLocalConfig + ?? (await pathExists(existingConfigPath) ? null : await loadConfigSetAside(existingConfigPath)); let inheritUserScope: boolean | undefined; try { inheritUserScope = resolveInheritUserScope( scope, options.inheritUserScope, - existingLocalConfig?.inheritUserScope, + carriedConfig?.inheritUserScope, ); } catch (e) { log.error((e as Error).message); @@ -1365,9 +1438,6 @@ export async function init(options: GlobalOptions & { if (fallbackReason) { log.warn(fallbackReason); } - const teamaiHome = scope === 'project' && projectRoot - ? (existingLocalConfig?.dataHome ?? await resolveProjectDataHome(projectRoot)) - : getTeamaiHome(scope, projectRoot); printScopeSummary(scope, projectRoot, explicit); if (scope === 'project' && !(await isInsideGitRepo(process.cwd()))) { @@ -1375,7 +1445,6 @@ export async function init(options: GlobalOptions & { } // Step 0.5: Re-init guard — warn if config already exists - const existingConfigPath = path.join(teamaiHome, 'config.yaml'); if (await pathExists(existingConfigPath)) { log.warn(`teamai is already initialized for ${scope} scope at ${existingConfigPath}`); if (options.force) { @@ -1452,6 +1521,24 @@ export async function init(options: GlobalOptions & { // Step 3: Clone or link repo const defaultLocalPath = path.join(teamaiHome, 'team-repo'); const localPath = expandHome(defaultLocalPath); + // The clone init uses is another install's than the config beside it, whether + // it clones it now or reuses one an earlier init left: settle that install + // now, as the config save below would, instead of leaving its config to run + // against this clone should init stop first. + const settleReplacedInstall = async (): Promise<void> => { + const next: LocalConfig = { + repo: { localPath, remote: repoInfo.httpsUrl }, + username, + scope, + projectRoot, + additionalRoles: [], + ...(scope === 'project' ? { dataHome: teamaiHome } : {}), + }; + const replacesAnother = existingLocalConfig + ? !sameQueueOwner(queueOwner(existingLocalConfig), queueOwner(next)) + : await pathExists(existingConfigPath); + if (replacesAnother) await settleModeSwitch(existingLocalConfig, next, () => moveConfigAside(existingConfigPath, next)); + }; if (await pathExists(localPath)) { if (await isGitRepo(localPath)) { @@ -1485,6 +1572,7 @@ export async function init(options: GlobalOptions & { } } else { log.info(`Repo already exists at ${localPath}, using existing clone`); + await settleReplacedInstall(); // Refresh before resolveActiveProjects so selectors like `--project all` // expand against the current remote manifest, not a stale local snapshot // (re-running init after a new project is added would otherwise keep the @@ -1518,6 +1606,8 @@ export async function init(options: GlobalOptions & { } if (!await pathExists(localPath)) { + await settleReplacedInstall(); + const cloneSpin = spinner('Cloning team repo...').start(); const cloneTarget = provider.name === 'git' ? repoInfo.httpsUrl @@ -1817,10 +1907,14 @@ export async function init(options: GlobalOptions & { // Persist --agent into enabledAgents (additive across runs) const requestedAgents = normalizeAgentList(options.agent); if (requestedAgents.length > 0) { - const existing = await loadLocalConfigForScope(scope, projectRoot); - const prev = existing?.enabledAgents ?? []; + // As loaded before the clone: that config may have been moved aside since. + const prev = carriedConfig?.enabledAgents ?? []; localConfig.enabledAgents = [...new Set([...prev, ...requestedAgents])]; - localConfig.disabledAgents = (existing?.disabledAgents ?? []).filter((t) => !requestedAgents.includes(t)); + localConfig.disabledAgents = (carriedConfig?.disabledAgents ?? []).filter((t) => !requestedAgents.includes(t)); + } else { + // No --agent: the lists stand as they were, `uninstall --agent`'s exclusion included. + if (carriedConfig?.enabledAgents) localConfig.enabledAgents = [...carriedConfig.enabledAgents]; + if (carriedConfig?.disabledAgents) localConfig.disabledAgents = [...carriedConfig.disabledAgents]; } // Carry the member's recorded tool roots across a re-init. `init` is @@ -1830,11 +1924,11 @@ export async function init(options: GlobalOptions & { // entry when the variable IS set. // A project-scope config with no record of its own starts from the user-scope // one: the root is a fact about this machine, and project hooks land in HOME. - const carriedToolRoots = existingLocalConfig?.toolRoots + const carriedToolRoots = carriedConfig?.toolRoots ?? (scope === 'project' ? (await loadLocalConfigForScope('user'))?.toolRoots : undefined); if (carriedToolRoots) localConfig.toolRoots = { ...carriedToolRoots }; recordClaudeConfigRoot(localConfig); - await releasePreviousClaudeRoot(currentConfig, existingLocalConfig, localConfig); + await releasePreviousClaudeRoot(currentConfig, carriedConfig, localConfig); await ensureDir(teamaiHome); if (scope !== 'project') await ensureDir(getTeamaiHomeDir()); @@ -1847,27 +1941,7 @@ export async function init(options: GlobalOptions & { // Generate .gitignore for project scope to prevent local config from being committed const gitignorePath = path.join(teamaiHome, '.gitignore'); if (!await pathExists(gitignorePath)) { - const gitignoreContent = [ - '# teamai local config (do not commit)', - 'config.yaml', - 'state.json', - 'token', - 'teamai.lock', - '.update-lock', - 'env', - 'env.sh', - 'sessions/', - 'dashboard/', - 'usage.jsonl', - 'usage.jsonl.*', - 'usage.pending-*.jsonl', - 'known-skills.json', - 'learnings/', - 'search-index.json', - 'votes/', - '', - ].join('\n'); - await writeFile(gitignorePath, gitignoreContent); + await writeFile(gitignorePath, buildProjectScopeGitignore()); log.debug('Generated .teamai/.gitignore for project scope'); } } else { diff --git a/src/maintenance/confidence.ts b/src/maintenance/confidence.ts index 0136a3758..93e089213 100644 --- a/src/maintenance/confidence.ts +++ b/src/maintenance/confidence.ts @@ -88,14 +88,14 @@ export async function computeAllConfidence(votesDir: string): Promise<Map<string /** * Write confidence scores back into learning document frontmatter. * Only updates docs whose confidence changed by > 0.05. - * Returns count of files updated. + * Returns the files it wrote, which are what a publish may stage (#823). */ export async function writeBackConfidence( learningsDirs: readonly string[], confidenceMap: Map<string, number>, writeRoot?: string, -): Promise<number> { - let updated = 0; +): Promise<string[]> { + const written: string[] = []; const files = await listLearningFiles(learningsDirs); for (const { file, absPath } of files) { @@ -123,14 +123,14 @@ export async function writeBackConfidence( : absPath; await ensureDir(path.dirname(target)); await writeFile(target, newContent); - updated++; + written.push(target); } catch { log.debug(`confidence: failed to update frontmatter for: ${file}`); } } - if (updated > 0) { - log.info(`Updated confidence scores for ${updated} learning(s)`); + if (written.length > 0) { + log.info(`Updated confidence scores for ${written.length} learning(s)`); } - return updated; + return written; } diff --git a/src/maintenance/promote.ts b/src/maintenance/promote.ts index 0e5b187f3..11c21069e 100644 --- a/src/maintenance/promote.ts +++ b/src/maintenance/promote.ts @@ -148,11 +148,15 @@ Output ONLY the transformed markdown content (including YAML frontmatter with ti } } +/** + * The promoted entry's path, and the learning file it marked as promoted (null + * when it marked none), which is what a publish may stage (#823). + */ export async function executePromotion( candidate: PromotionCandidate, repoPath: string, options: PromoteOptions & { learningsWriteDir?: string } = {}, -): Promise<string> { +): Promise<{ targetPath: string; marked: string | null }> { const category = options.category ?? candidate.suggestedCategory; const targetDir = path.join(repoPath, category); await ensureDir(targetDir); @@ -161,13 +165,13 @@ export async function executePromotion( if (options.dryRun) { log.info(`[dry-run] Would promote ${candidate.docId} -> ${category}/${candidate.filename}`); - return targetPath; + return { targetPath, marked: null }; } const originalContent = await readFileSafe(candidate.path); if (!originalContent) { log.error(`Cannot read source file: ${candidate.path}`); - return targetPath; + return { targetPath, marked: null }; } // AI transforms the learning into a generalized format for the target category @@ -189,7 +193,7 @@ export async function executePromotion( await writeFile(markPath, updated); log.success(`Promoted: ${candidate.docId} -> ${category}/${candidate.filename}`); - return targetPath; + return { targetPath, marked: markPath }; } function inferCategoryByKeywords(content: string, title: string): 'skills' | 'rules' | 'docs' { diff --git a/src/maintenance/prune.ts b/src/maintenance/prune.ts index 1ed70076d..971b87eb5 100644 --- a/src/maintenance/prune.ts +++ b/src/maintenance/prune.ts @@ -81,19 +81,21 @@ export async function findPruneCandidates( } /** - * Execute prune: archive or remove low-confidence documents. + * Execute prune: archive or remove low-confidence documents. `changed` is every + * file it removed or wrote, which is what a publish may stage (#823). */ export async function executePrune( learningsWriteDir: string, candidates: PruneCandidate[], options: PruneOptions = {}, -): Promise<{ archived: number; removed: number }> { +): Promise<{ archived: number; removed: number; changed: string[] }> { let archived = 0; let removed = 0; + const changed: string[] = []; if (options.dryRun) { log.info(`[dry-run] Would ${options.archive ? 'archive' : 'remove'} ${candidates.length} file(s)`); - return { archived: 0, removed: 0 }; + return { archived: 0, removed: 0, changed }; } const inherited: string[] = []; @@ -109,11 +111,14 @@ export async function executePrune( if (options.archive) { const archiveDir = path.join(learningsWriteDir, '_archive'); await ensureDir(archiveDir); - await copyFile(candidate.path, path.join(archiveDir, candidate.filename)); + const archivedCopy = path.join(archiveDir, candidate.filename); + await copyFile(candidate.path, archivedCopy); await remove(candidate.path); + changed.push(candidate.path, archivedCopy); archived++; } else { await remove(candidate.path); + changed.push(candidate.path); removed++; } } @@ -126,5 +131,5 @@ export async function executePrune( + `${inherited.join(', ')}. Remove them with a pull request against the team repo.`, ); } - return { archived, removed }; + return { archived, removed, changed }; } diff --git a/src/migrate.ts b/src/migrate.ts index 365a32b5d..c8aa01c43 100644 --- a/src/migrate.ts +++ b/src/migrate.ts @@ -439,7 +439,9 @@ export async function runMigration( await releaseLock(lockPath); lockReleased = true; const backup = await retireLegacy(legacyDir); - log.success(`Finished an interrupted migration: retired ${legacyDir} to ${backup}`); + // Not always an interrupted run: a linked worktree lands here once another + // checkout of the repo built the partition. + log.success(`Retired ${legacyDir} to ${backup}: this project's data already lives in ${partitionDir}`); return 'migrated'; } @@ -881,7 +883,7 @@ async function migrateSelfA1(legacyDir: string, partitionDir: string, legacyOwne * - a config that cannot be read: left in place, since where they would be * published is unknown. */ -export async function settleCheckoutQueue(legacyDir: string, partitionDir: string, owner: QueueOwner): Promise<void> { +async function settleCheckoutQueue(legacyDir: string, partitionDir: string, owner: QueueOwner): Promise<void> { const queue = path.join(legacyDir, SELF_LEGACY_QUEUE); if (!(await pathExists(queue))) return; // The install read below decides where the queue goes; an init switching it diff --git a/src/pull.ts b/src/pull.ts index 2faf84926..31455496f 100644 --- a/src/pull.ts +++ b/src/pull.ts @@ -752,11 +752,13 @@ async function pullForScope( // partition sync lock across this scope and the lock is not reentrant, so // publishing must not try to take it again. Never let it block the pull. try { - const queue = await publishQueuedLearnings(localConfig, localConfig.username, { holdsSyncLock: true }); - if (queue.published.length > 0) { + const queue = await publishQueuedLearnings(localConfig, localConfig.username, { holdsSyncLock: true, dryRun: options.dryRun }); + if (options.dryRun) { + if (queue.remaining > 0) log.info(`[${scopeLabel}] [dry-run] Would publish ${queue.remaining} queued learning(s)`); + } else if (queue.published.length > 0) { log.success(`Published ${queue.published.length} queued learning(s)`); } - if (queue.remaining > 0) { + if (queue.remaining > 0 && !options.dryRun) { // Say it out loud. A member whose pushes are rejected would otherwise // queue notes forever and never hear about it. reported.add('pending-learnings'); diff --git a/src/utils/branch-worktree.ts b/src/utils/branch-worktree.ts index 8d9d45ad4..163ef63b9 100644 --- a/src/utils/branch-worktree.ts +++ b/src/utils/branch-worktree.ts @@ -136,7 +136,7 @@ async function remoteBranchExists(spec: BranchWorktreeSpec, repoRoot: string): P * stale. An explicit refspec creates and updates that tracking ref in every * clone (#706). Used by both the reports and learnings branches. */ -async function fetchTrackingRef(git: SimpleGit, branch: string): Promise<void> { +export async function fetchTrackingRef(git: SimpleGit, branch: string): Promise<void> { await git.fetch(['origin', `+refs/heads/${branch}:refs/remotes/origin/${branch}`]); } @@ -274,12 +274,7 @@ async function ensureWorktree( */ async function removeOldCheckoutsInDotTeamai(spec: BranchWorktreeSpec, git: SimpleGit, wt: string): Promise<void> { const oldSuffix = `${path.sep}${path.join('.teamai', spec.worktreeDirname)}`; - const listing = await git.raw(['worktree', 'list', '--porcelain']); - for (const entry of listing.split('\n\n')) { - const lines = entry.split('\n'); - const checkout = lines.find((l) => l.startsWith('worktree '))?.slice('worktree '.length); - const branch = lines.find((l) => l.startsWith('branch '))?.slice('branch '.length); - if (checkout === undefined || branch !== `refs/heads/${spec.branch}`) continue; + for (const checkout of await checkoutsOfBranch(spec, git)) { // resolve: git prints forward slashes on Windows too. if (!path.resolve(checkout).endsWith(oldSuffix) || path.resolve(checkout) === path.resolve(wt)) continue; try { @@ -298,6 +293,31 @@ async function removeOldCheckoutsInDotTeamai(spec: BranchWorktreeSpec, git: Simp } } +/** The checkouts of this branch the repository behind `git` registers (git allows one). */ +async function checkoutsOfBranch(spec: BranchWorktreeSpec, git: SimpleGit): Promise<string[]> { + const listing = await git.raw(['worktree', 'list', '--porcelain']); + const checkouts: string[] = []; + for (const entry of listing.split('\n\n')) { + const lines = entry.split('\n'); + const checkout = lines.find((l) => l.startsWith('worktree '))?.slice('worktree '.length); + const branch = lines.find((l) => l.startsWith('branch '))?.slice('branch '.length); + if (checkout !== undefined && branch === `refs/heads/${spec.branch}`) checkouts.push(checkout); + } + return checkouts; +} + +/** + * Where this repository has the branch checked out, wherever that is: the + * shared checkout, or one an older teamai left in a checkout's `.teamai/`. + * Null when it has none, or no branch worktrees at all. Never another + * repository's checkout, since it is this repository's own registration. + */ +async function registeredCheckoutImpl(spec: BranchWorktreeSpec, localConfig: LocalConfig): Promise<string | null> { + if (!usesBranchWorktree(localConfig)) return null; + const [checkout] = await checkoutsOfBranch(spec, createGit(gitRoot(localConfig))); + return checkout ?? null; +} + /** * Self and git mode keep this checkout at the same path in the partition * (#808), so after a project switches mode, the checkout there may belong to @@ -528,9 +548,15 @@ async function commitAndPushAt( ): Promise<PublishResult> { const git = createGit(wt); - await git.add(files); - const status = await git.status(); - if (status.staged.length === 0 && !options.pushIfUnchanged && await nothingLeftToPush(git, spec)) { + // Literal: a filename with `[` or `*` would otherwise stage whatever it matches as a pattern. + await git.raw(['--literal-pathspecs', 'add', '--', ...files]); + // Only `files`: the checkout may hold a file someone else staged, and it must + // neither count as a change nor ride along in this commit. No paths would be the whole index. + const staged = files.length === 0 + ? 0 + : (await git.raw(['--literal-pathspecs', 'diff', '--cached', '--no-renames', '--name-only', '-z', '--', ...files])) + .split('\0').filter(Boolean).length; + if (staged === 0 && !options.pushIfUnchanged && await nothingLeftToPush(git, spec)) { // Nothing to commit AND nothing to deliver. Those are two different things: // an earlier attempt may have committed exactly this content and failed to // push it, and a caller that reads "already present" drops the only durable @@ -541,7 +567,7 @@ async function commitAndPushAt( // A retry may reconstruct the same tree as a previously committed // but unconfirmed push. It still needs a push, without an empty commit. - if (status.staged.length > 0) await commitSkippingHooks(git, message); + if (staged > 0) await commitSkippingHooks(git, message, files); // Push with fetch+rebase retry. Each member only writes <user>.yaml, so // rebase conflicts are effectively impossible; retries handle the pure @@ -559,8 +585,13 @@ async function commitAndPushAt( log.debug(`[${spec.logTag}] push failed after ${attempt} attempts: ${(pushErr as Error).message}`); return { status: 'failed', reason: (pushErr as Error).message }; } + // The commit took only `files`: anything else staged or modified would + // make git refuse to rebase, on this attempt and every later one. + let carried: string | null = null; try { await fetchTrackingRef(git, spec.branch); + carried = await snapshotDirtyTree(spec, git); + if (carried) await git.raw(['reset', '--hard', 'HEAD']); await git.rebase([`origin/${spec.branch}`]); } catch (rebaseErr) { log.debug(`[${spec.logTag}] rebase failed, retrying: ${(rebaseErr as Error).message}`); @@ -571,6 +602,7 @@ async function commitAndPushAt( // no rebase in progress } } + if (carried) await applyDirtySnapshot(spec, git, carried); } } return { status: 'failed', reason: `push did not land after ${MAX_PUSH_RETRIES} attempts` }; @@ -711,9 +743,11 @@ async function restoreConflictedFiles(spec: BranchWorktreeSpec, git: SimpleGit): * returns a dangling commit). `git rebase --autostash` would push onto the * shared stash list, which other worktrees of this repo also see. */ -async function snapshotDirtyTree(git: SimpleGit): Promise<string | null> { +async function snapshotDirtyTree(spec: BranchWorktreeSpec, git: SimpleGit): Promise<string | null> { try { const sha = (await git.raw(['stash', 'create'])).trim(); + // In debug.log, so a run killed before the snapshot is applied again can be recovered by hand. + if (sha.length > 0) log.debug(`[${spec.logTag}] uncommitted files saved as ${sha}; git stash apply --index ${sha} restores them`); return sha.length > 0 ? sha : null; } catch { return null; @@ -722,11 +756,127 @@ async function snapshotDirtyTree(git: SimpleGit): Promise<string | null> { async function applyDirtySnapshot(spec: BranchWorktreeSpec, git: SimpleGit, sha: string): Promise<void> { try { - await git.raw(['stash', 'apply', sha]); + // `--index` keeps what was staged staged. It refuses, touching nothing, + // when the staged changes no longer apply; then restore the files alone. + await git.raw(['stash', 'apply', '--index', sha]); + return; } catch { - // apply conflicts leave UU paths; restoreConflictedFiles recovers them + try { + await git.raw(['stash', 'apply', sha]); + } catch { + // apply conflicts leave unmerged paths; restoreConflictsFromSnapshot resolves them + } + } + const conflicted = await restoreConflictsFromSnapshot(spec, git, sha); + if (conflicted !== null) await restageSnapshotIndex(spec, git, sha, conflicted); +} + +/** + * Resolve each stash-apply conflict to the snapshot's own copy of the path, + * left unstaged: its file, or no file where the snapshot had removed it (the + * version that removes is origin's, which HEAD holds). Unlike + * restoreConflictedFiles this needs no `--theirs` side, which a removal lacks, + * and on failure it resets nothing: it names the snapshot to restore from. + * Returns the conflicted paths, or null when they could not be resolved. + */ +async function restoreConflictsFromSnapshot(spec: BranchWorktreeSpec, git: SimpleGit, sha: string): Promise<string[] | null> { + let conflicted: string[] = []; + try { + conflicted = (await git.status()).conflicted ?? []; + if (conflicted.length === 0) return []; + const inSnapshot = new Set((await git.raw(['--literal-pathspecs', 'ls-tree', '-z', '--name-only', sha, '--', ...conflicted])).split('\0').filter(Boolean)); + const kept = conflicted.filter((p) => inSnapshot.has(p)); + const removed = conflicted.filter((p) => !inSnapshot.has(p)); + if (kept.length > 0) await git.raw(['--literal-pathspecs', 'checkout', sha, '--', ...kept]); + if (removed.length > 0) await git.raw(['--literal-pathspecs', 'rm', '-q', '-f', '--', ...removed]); + await git.raw(['--literal-pathspecs', 'reset', '-q', 'HEAD', '--', ...conflicted]); + return conflicted; + } catch (e) { + const wt = await git.raw(['rev-parse', '--show-toplevel']).then((out) => out.trim(), () => spec.worktreeDirname); + log.warn( + `Could not restore uncommitted files in ${wt} after updating from origin (${failureReason(e)})` + + (conflicted.length > 0 ? `; ${conflicted.join(', ')} may hold conflict markers` : '') + + `. Nothing was discarded: commit ${sha} holds them as they were; read one with git -C "${wt}" show ${sha}:<file>, ` + + `or run git -C "${wt}" stash apply --index ${sha} once the conflicts are cleared.`, + ); + return null; + } +} + +/** A tree entry, as `ls-tree` prints it. */ +type TreeEntry = { mode: string; id: string }; + +/** + * After a plain `stash apply`, put back what the snapshot had staged, and only + * where that is provably the same staging: + * - origin did not change the path: its staged entry is restored as it was, + * so a file staged in part stays staged in part; + * - origin changed it, the apply merged without a conflict, and the file holds + * exactly what was staged: staged again. + * Anything else stays unstaged, its content untouched, and is named in a + * warning: staging a conflicted path again would stage a revert of origin's change. + */ +async function restageSnapshotIndex(spec: BranchWorktreeSpec, git: SimpleGit, sha: string, conflicted: readonly string[]): Promise<void> { + let wt: string; + let paths: string[]; + let base: Map<string, TreeEntry>; + let staged: Map<string, TreeEntry>; + let head: Map<string, TreeEntry>; + try { + wt = (await git.raw(['rev-parse', '--show-toplevel'])).trim(); + // A `stash create` commit: first parent is HEAD at the time, second parent is the index. + paths = (await git.raw(['diff', '--name-only', '--no-renames', '-z', `${sha}^1`, `${sha}^2`])).split('\0').filter(Boolean); + if (paths.length === 0) return; + const entries = async (rev: string): Promise<Map<string, TreeEntry>> => { + const listing = await git.raw(['--literal-pathspecs', 'ls-tree', '-z', rev, '--', ...paths]); + // `<mode> <type> <id>\t<path>` + return new Map(listing.split('\0').filter(Boolean).map((line) => { + const [meta, p] = line.split('\t'); + const [mode, , id] = meta.split(' '); + return [p, { mode, id }]; + })); + }; + base = await entries(`${sha}^1`); + staged = await entries(`${sha}^2`); + head = await entries('HEAD'); + } catch (e) { + log.warn( + `Could not tell which files were staged before updating from origin (${failureReason(e)}); ` + + `a staged change may now be unstaged, its content kept. git stash apply --index ${sha} in that checkout restores the staging.`, + ); + return; + } + + const stage = async (p: string, entry: TreeEntry | undefined, exact: boolean): Promise<void> => { + if (entry === undefined) await git.raw(['update-index', '--force-remove', '--', p]); + else if (exact) await git.raw(['update-index', '--add', '--cacheinfo', `${entry.mode},${entry.id},${p}`]); + else await git.raw(['--literal-pathspecs', 'add', '--', p]); + }; + const sameEntry = (a: TreeEntry | undefined, b: TreeEntry | undefined): boolean => a?.mode === b?.mode && a?.id === b?.id; + + const unstaged: string[] = []; + for (const p of paths) { + const entry = staged.get(p); + try { + if (sameEntry(head.get(p), base.get(p))) { + await stage(p, entry, true); + continue; + } + const abs = path.join(wt, p); + const current = (await pathExists(abs)) ? (await git.raw(['hash-object', '--', abs])).trim() : undefined; + if (conflicted.includes(p) || current !== entry?.id) unstaged.push(p); + else await stage(p, entry, false); + } catch (e) { + log.debug(`[${spec.logTag}] cannot stage ${p} again: ${failureReason(e)}`); + unstaged.push(p); + } + } + if (unstaged.length > 0) { + log.warn( + `Origin changed files that were staged in ${wt}: ${unstaged.join(', ')}. They keep your content, unstaged. ` + + `Compare them with origin's version using git -C "${wt}" diff HEAD, then stage what you mean to keep.`, + ); } - await restoreConflictedFiles(spec, git); } /** @@ -766,7 +916,7 @@ async function syncWorktree(spec: BranchWorktreeSpec, wt: string): Promise<void> let carried: string | null = null; if (dirty && ahead > 0) { - carried = await snapshotDirtyTree(git); + carried = await snapshotDirtyTree(spec, git); if (!carried) { log.debug(`[${spec.logTag}] uncommitted files block the refresh; using the local copy`); return; @@ -890,6 +1040,8 @@ export interface BranchWorktree { checkOwner(localConfig: LocalConfig): Promise<void>; /** True when the checkout there is not provably this repository's; reads git's files, runs no git. */ isForeignByFiles(localConfig: LocalConfig): Promise<boolean>; + /** Where this repository has the branch checked out, the old `.teamai/` place included; creates nothing. */ + registeredCheckout(localConfig: LocalConfig): Promise<string | null>; } /** @@ -918,5 +1070,6 @@ export function createBranchWorktree(spec: BranchWorktreeSpec): BranchWorktree { refresh: (localConfig, options) => refreshImpl(spec, localConfig, options), checkOwner: (localConfig) => checkOwnerImpl(spec, localConfig), isForeignByFiles: (localConfig) => isForeignByFilesImpl(spec, localConfig), + registeredCheckout: (localConfig) => registeredCheckoutImpl(spec, localConfig), }; } diff --git a/src/utils/git.ts b/src/utils/git.ts index 9166241cd..00c06ac41 100644 --- a/src/utils/git.ts +++ b/src/utils/git.ts @@ -29,9 +29,14 @@ export function createGit(basePath?: string): SimpleGit { * * `--no-verify` is scoped to this git process. It does not write * `core.hooksPath` and does not change the user's ordinary `git commit`. + * + * With `paths`, commits those paths alone, taken literally, and leaves anything + * else staged in the checkout out of the commit and still staged. Without, it + * commits the whole index. */ -export function commitSkippingHooks(git: SimpleGit, message: string) { - return git.commit(message, { '--no-verify': null }); +export function commitSkippingHooks(git: SimpleGit, message: string, paths?: readonly string[]) { + if (paths === undefined) return git.commit(message, { '--no-verify': null }); + return git.raw(['--literal-pathspecs', 'commit', '--no-verify', '-m', message, '--', ...paths]); } /** diff --git a/src/utils/learnings-publish.ts b/src/utils/learnings-publish.ts index 25ac5d67d..51d47ffc4 100644 --- a/src/utils/learnings-publish.ts +++ b/src/utils/learnings-publish.ts @@ -6,18 +6,25 @@ * the team later instead of being lost. A queue entry is dropped only once its * content is confirmed on origin, so a failure of any kind is always safe. */ +import { createHash } from 'node:crypto'; import path from 'node:path'; import fs from 'node:fs'; +import type { SimpleGit } from 'simple-git'; +import { z } from 'zod'; -import { ensureDir } from './fs.js'; +import { ensureDir, writeFileAtomic } from './fs.js'; +import { parseFrontmatter } from './frontmatter.js'; +import { createGit } from './git.js'; import { learningsBranch } from './learnings-branch.js'; -import { CheckoutRefusedError, failureReason, type PublishResult } from './branch-worktree.js'; +import { CheckoutRefusedError, failureReason, fetchTrackingRef, type PublishResult } from './branch-worktree.js'; import { log } from './logger.js'; import { dropPendingLearning, listPendingForInstall, listPendingLearnings, + readPendingForInstall, readPendingLearning, + savePendingLearning, } from './pending-learnings.js'; import { acquireLock, releaseLock } from '../update.js'; import { getDataHome, SYNC_LOCK_FILENAME, type LocalConfig } from '../types.js'; @@ -57,8 +64,32 @@ function commitMessageFor(username: string): string { export async function publishQueuedLearnings( localConfig: LocalConfig, username: string, - options: { holdsSyncLock?: boolean } = {}, + options: { holdsSyncLock?: boolean; dryRun?: boolean } = {}, ): Promise<PublishQueueReport> { + // Publishing writes to the team clone, which `pull` and `push` guard with the + // partition sync lock. On contention nothing is lost and nothing is forced: + // the learnings stay queued and the run that holds the lock publishes them. + // `pull` already holds the lock when it calls this, and the lock is not + // reentrant, so it says so instead of deadlocking against itself. + const syncLock = options.holdsSyncLock ? null : syncLockPath(localConfig); + const locked = syncLock === null || await acquireLock(syncLock); + try { + return await publishUnderSyncLock(localConfig, username, locked, options.dryRun === true); + } finally { + if (syncLock && locked) await releaseLock(syncLock); + } +} + +async function publishUnderSyncLock( + localConfig: LocalConfig, + username: string, + locked: boolean, + dryRun: boolean, +): Promise<PublishQueueReport> { + // Before the queue is read, which this adds to. Only under the sync lock: + // two commands at once would each queue the same file. + if (locked && !dryRun) await queueImportRemnants(localConfig); + const listing = await listPendingForInstall(localConfig); switch (listing.status) { case 'listed': @@ -80,18 +111,20 @@ export async function publishQueuedLearnings( throw new Error(`Unhandled queue listing: ${JSON.stringify(unhandled)}`); } } + // What a maintenance run changed and could not publish goes first, and + // whether or not anything is queued: rerunning maintenance finds nothing to change. + if (locked && !dryRun) { + const maintenance = await publishRecordedMaintenance(localConfig); + if (maintenance.status === 'published') log.success('Published earlier maintenance changes to the learnings branch'); + else if (maintenance.status !== 'already-present') log.debug(`[learnings] recorded maintenance changes stay local: ${maintenance.status}`); + } + const queued = listing.queued; if (queued.length === 0) { return { published: [], remaining: 0 }; } - - // Publishing writes to the team clone, which `pull` and `push` guard with the - // partition sync lock. On contention nothing is lost and nothing is forced: - // the learnings stay queued and the run that holds the lock publishes them. - // `pull` already holds the lock when it calls this, and the lock is not - // reentrant, so it says so instead of deadlocking against itself. - const syncLock = options.holdsSyncLock ? null : syncLockPath(localConfig); - if (syncLock && !(await acquireLock(syncLock))) { + if (dryRun) return { published: [], remaining: queued.length }; + if (!locked) { log.debug('[learnings] a pull or push is in progress; leaving the queue for it'); return { published: [], @@ -117,8 +150,6 @@ export async function publishQueuedLearnings( lastError: failureReason(e), refused: e instanceof CheckoutRefusedError || undefined, }; - } finally { - if (syncLock) await releaseLock(syncLock); } } @@ -137,21 +168,335 @@ function syncLockPath(localConfig: LocalConfig): string | null { } /** - * Publish whatever maintenance just changed in the learnings worktree. + * How `import --from-mr` in 0.25.0 to 0.26.0-beta.3 named the learning it wrote: + * `<YYYY-MM-DD>-<title>.md`, the title part empty when no character of it was kept. + */ +const IMPORT_REMNANT_NAME = /^\d{4}-\d{2}-\d{2}-(.*)\.md$/; + +/** The merge request a learning was extracted from, from its frontmatter. */ +function sourceMr(frontmatter: Record<string, unknown>): string | null { + const mr = frontmatter.source_mr; + return typeof mr === 'string' && mr.trim() !== '' ? mr.trim() : null; +} + +/** + * Queue what `import --from-mr` (0.25.0 to 0.26.0-beta.3) wrote into the learnings checkout and + * never committed (#823 item 7), so the publish that follows sends it. Such a + * file never reached the team and, in single-repo mode, keeps git from removing + * the checkout an older teamai left in `.teamai/`. + * + * Only that exact shape moves: untracked, directly under `learnings/`, named + * `<date>-<title>.md`, with `source_mr` in its frontmatter. Nothing else in the + * checkout is touched; a learning on the branch is tracked, edited or not, so it + * never is. One is removed instead when origin's branch or the queue already has one + * from the same merge request (a later import of it) or with the same content. The copy is queued before the original goes, so + * a failure leaves it where it was. Never throws. + */ +async function queueImportRemnants(localConfig: LocalConfig): Promise<void> { + try { + const checkout = await learningsBranch.registeredCheckout(localConfig); + if (checkout === null) return; + const git = createGit(checkout); + const lsFiles = async (args: string[]): Promise<string[]> => + (await git.raw(['ls-files', '-z', ...args, '--', 'learnings'])).split('\0').filter(Boolean); + + const remnants: Array<{ file: string; content: string; mr: string; title?: string }> = []; + for (const rel of await lsFiles(['--others', '--exclude-standard'])) { + // Directly under `learnings/`: those versions wrote nothing into a namespace. + if (rel.split('/').length !== 2) continue; + const named = IMPORT_REMNANT_NAME.exec(path.posix.basename(rel)); + if (!named) continue; + const file = path.join(checkout, rel); + // One that cannot be read, such as a dangling link, must not hold back the rest on every run. + const content = await fs.promises.readFile(file, 'utf-8').catch((e: unknown) => { + log.debug(`[learnings] cannot read ${file}: ${failureReason(e)}`); + return null; + }); + if (content === null) continue; + const { data } = parseFrontmatter(content); + const mr = sourceMr(data); + if (mr === null) continue; + const title = typeof data.title === 'string' && data.title.trim() ? data.title : named[1]; + remnants.push({ file, content, mr, title: title || undefined }); + } + if (remnants.length === 0) return; + + // What already covers a remnant: a learning on origin, in any namespace, + // or one in the queue. Origin itself, fetched just now, not the checkout: + // the one an older teamai left in `.teamai/` is never synced again (#823 + // item 21), so it may lack what a teammate published since, or still track + // a copy origin has deleted, and a remnant removed against that copy is + // lost. Without a current origin every remnant stays for a later run. + const known = await learningsOnOrigin(git); + if (known === null) return; + // The queue only while it is still this install's: after init switched the + // project to another team repository, it holds that install's learnings, and + // a remnant removed against one of them never reached this one's repository. + const queue = await readPendingForInstall(localConfig); + if (queue.status !== 'read') { + log.debug(`[learnings] leaving what an older import --from-mr left for a later run: the queue is ${queue.status}`); + return; + } + for (const { relPath, content } of queue.queued) { + known.push({ label: `the contribution queue (${relPath})`, content, mr: sourceMr(parseFrontmatter(content).data) }); + } + + const { generateFilename, resolveLearningsSubdir } = await import('../contribute.js'); + const subdir = await resolveLearningsSubdir(localConfig); + const queued: string[] = []; + for (const remnant of remnants) { + const covered = known.find((k) => k.content === remnant.content || k.mr === remnant.mr); + if (covered) { + if (!await removeRemnant(remnant.file)) continue; + log.warn(`Removed ${remnant.file}, which an older teamai import --from-mr left unpublished: ${covered.label} already has it.`); + continue; + } + const relPath = path.posix.join(subdir, generateFilename(remnant.title)); + const saved = await savePendingLearning(localConfig, relPath, remnant.content); + if (saved.status !== 'saved') { + log.debug(`[learnings] could not queue ${remnant.file}: ${saved.status}`); + break; + } + // Queued even when it cannot go: the next run finds the queued copy and removes it then. + await removeRemnant(remnant.file); + queued.push(remnant.file); + known.push({ label: `the contribution queue (${relPath})`, content: remnant.content, mr: remnant.mr }); + } + if (queued.length > 0) { + log.warn(`Queued ${queued.length} learning(s) an older teamai import --from-mr left unpublished: ${queued.join(', ')}`); + } + } catch (e) { + log.debug(`[learnings] could not queue what an older import --from-mr left: ${failureReason(e)}`); + } +} + +/** Whether the remnant is gone; a failure is logged and left for a later run. */ +async function removeRemnant(file: string): Promise<boolean> { + try { + await fs.promises.rm(file, { force: true }); + return true; + } catch (e) { + log.debug(`[learnings] cannot remove ${file}: ${failureReason(e)}`); + return false; + } +} + +/** A learning that already covers a remnant, and where it is. */ +interface KnownLearning { + label: string; + content: string; + mr: string | null; +} + +/** + * Every learning on origin's learnings branch, fetched just now. None when + * origin has no learnings branch, as after an offline first publish. Null when + * origin cannot be reached or read. + */ +async function learningsOnOrigin(git: SimpleGit): Promise<KnownLearning[] | null> { + const ref = `origin/${learningsBranch.branch}`; + try { + try { + await fetchTrackingRef(git, learningsBranch.branch); + } catch (e) { + // A fetch of a branch origin lacks fails too; ls-remote tells the two + // apart, succeeding with no output only when origin answered without it. + if ((await git.raw(['ls-remote', '--heads', 'origin', `refs/heads/${learningsBranch.branch}`])).trim() !== '') throw e; + return []; + } + const files = (await git.raw(['ls-tree', '-r', '-z', '--name-only', ref, '--', 'learnings'])).split('\0').filter((f) => f.endsWith('.md')); + const learnings: KnownLearning[] = []; + for (const rel of files) { + const content = await git.show([`${ref}:${rel}`]); + learnings.push({ label: rel, content, mr: sourceMr(parseFrontmatter(content).data) }); + } + return learnings; + } catch (e) { + log.debug(`[learnings] cannot read ${ref}; leaving what an older import --from-mr left for a later run: ${failureReason(e)}`); + return null; + } +} + +/** + * Publish whatever maintenance just changed in the learnings worktree, and + * whatever an earlier run changed and could not publish. * * Pruning, promotion and confidence write-backs used to mutate a checkout * nothing pushes, so their result reached no teammate and the next realign * could undo it. They now write into the worktree, and this is what makes the - * change leave the machine. + * change leave the machine. The files are changed before the branch lock is + * taken, and a rerun finds nothing left to change, so each run records what it + * changed first and the record goes only once origin has it: a busy lock or a + * failed push leaves it for the next publish, contributions and pulls included. */ export async function publishLearningsMaintenance( localConfig: LocalConfig, message: string, + changed: readonly string[], ): Promise<PublishResult> { + const checkout = learningsBranch.dir(localConfig); + const inCheckout = changed + .map((file) => path.relative(checkout, file)) + .filter((rel) => rel !== '' && !rel.startsWith('..') && !path.isAbsolute(rel)); + if (inCheckout.length < changed.length) { + log.debug(`[learnings] maintenance changed files outside ${checkout}; not publishing those`); + } + if (!learningsBranch.enabled(localConfig)) { + // kind: 'http' has no branch to retry against. + return inCheckout.length === 0 ? { status: 'already-present' } : commitMaintenance(localConfig, message, inCheckout); + } + if (inCheckout.length > 0) { + try { + const contents: Record<string, string | null> = {}; + for (const rel of inCheckout) contents[rel] = await contentDigest(path.join(checkout, rel)); + await recordMaintenance(checkout, { message, files: inCheckout, contents }); + } catch (e) { + log.debug(`[learnings] cannot record maintenance changes for a retry: ${failureReason(e)}`); + return commitMaintenance(localConfig, message, inCheckout); + } + } + return publishRecordedMaintenance(localConfig); +} + +/** + * What one maintenance run changed, relative to the learnings checkout, and + * what it left in each file: a sha256, or null for a removal. Records from + * before `contents` existed publish their files as they are. + */ +const MaintenanceRecord = z.object({ + message: z.string(), + files: z.array(z.string()), + contents: z.record(z.string(), z.string().nullable()).optional(), +}); +type MaintenanceRecord = z.infer<typeof MaintenanceRecord>; + +async function hasUncommittedChange(checkout: string, rel: string): Promise<boolean> { + const status = await createGit(checkout).raw(['--literal-pathspecs', 'status', '--porcelain', '-z', '--untracked-files=all', '--', rel]); + return status.length > 0; +} + +async function contentDigest(file: string): Promise<string | null> { + if (!fs.existsSync(file)) return null; + return createHash('sha256').update(await fs.promises.readFile(file)).digest('hex'); +} + +/** + * Where maintenance records wait: in the checkout's own git directory, which + * git never shows or commits and which goes with the checkout, and with the + * changes they describe. One file per run, so a publish removes only what it read. + */ +async function maintenanceRecordsDir(checkout: string): Promise<string> { + const gitPath = (await createGit(checkout).raw(['rev-parse', '--git-path', 'teamai-maintenance'])).trim(); + return path.resolve(checkout, gitPath); +} + +async function recordMaintenance(checkout: string, record: MaintenanceRecord): Promise<void> { + const dir = await maintenanceRecordsDir(checkout); + const name = `${Date.now()}-${process.pid}-${Math.random().toString(36).slice(2, 10)}.json`; + // Atomic: a record cut short by a crash would otherwise be read on every publish. + await writeFileAtomic(path.join(dir, name), JSON.stringify(record)); +} + +/** Publish every recorded maintenance change, dropping the records once origin has them. Never throws. */ +async function publishRecordedMaintenance(localConfig: LocalConfig): Promise<PublishResult> { + const checkout = learningsBranch.dir(localConfig); + try { + if (!fs.existsSync(checkout)) return { status: 'already-present' }; + const dir = await maintenanceRecordsDir(checkout); + const records: Array<MaintenanceRecord & { file: string }> = []; + // `.json` only: an interrupted atomic write leaves a `.tmp` copy beside them. + for (const name of (await fs.promises.readdir(dir).catch(() => [])).filter((n) => n.endsWith('.json'))) { + const file = path.join(dir, name); + let text: string; + try { + text = await fs.promises.readFile(file, 'utf-8'); + } catch (e) { + // A failed read says nothing about the record: keep it for a later publish. + log.warn(`Cannot read maintenance record ${file} (${failureReason(e)}); it stays for the next publish.`); + continue; + } + let problem: string; + try { + const parsed = MaintenanceRecord.safeParse(JSON.parse(text)); + if (parsed.success) { + records.push({ ...parsed.data, file }); + continue; + } + problem = 'not a maintenance record'; + } catch (e) { + problem = failureReason(e); + } + // Read and malformed, so no later publish can use it either, and it must + // not hold back the others on every one. + await fs.promises.rm(file, { force: true }).catch(() => undefined); + log.warn( + `Removed unreadable maintenance record ${file} (${problem}). A maintenance change it described ` + + `that is not on the learnings branch stays uncommitted in ${checkout}; check with git -C "${checkout}" status.`, + ); + } + if (records.length === 0) return { status: 'already-present' }; + // A file publishes only as the newest run that changed it left it: an edit + // made since is someone's own, not this maintenance, and stays uncommitted. + // Names start with the time they were written. + records.sort((a, b) => (a.file < b.file ? -1 : a.file > b.file ? 1 : 0)); + const expected = new Map<string, string | null | undefined>(); + for (const record of records) for (const rel of record.files) expected.set(rel, record.contents?.[rel]); + const files: string[] = []; + const editedSince: string[] = []; + for (const [rel, digest] of expected) { + if (digest === undefined || (await contentDigest(path.join(checkout, rel))) === digest) files.push(rel); + // Nothing uncommitted: an earlier attempt committed the run's change, and a refresh merged origin's into it. + else if (!(await hasUncommittedChange(checkout, rel))) files.push(rel); + else editedSince.push(rel); + } + if (editedSince.length > 0) { + log.warn( + `Not publishing the edit to ${editedSince.join(', ')}: it was made after the maintenance run that changed ` + + `the file, so it is not part of that run, and no later teamai run publishes it. It stays uncommitted in ` + + `${checkout}; find it with git -C "${checkout}" status, then share it as a learning or discard it.`, + ); + } + const message = records.length === 1 ? records[0].message : `[teamai] Publish ${records.length} learnings maintenance runs`; + const result = await commitMaintenance(localConfig, message, files); + if (result.status === 'published' || result.status === 'already-present') { + for (const record of records) await fs.promises.rm(record.file, { force: true }); + } + return result; + } catch (e) { + return { status: 'failed', reason: failureReason(e) }; + } +} + +/** + * Commit and push `files`, relative to the learnings checkout. Also pushes what + * an earlier attempt committed, when none of them is left to stage. + */ +async function commitMaintenance(localConfig: LocalConfig, message: string, inCheckout: string[]): Promise<PublishResult> { + // Only the files maintenance wrote or removed, never all of `learnings/`: the + // checkout may hold files nobody committed, such as a learning an older + // import --from-mr left there, and they would ride along in this commit (#823). + const checkout = learningsBranch.dir(localConfig); + // A removed file git does not track has nothing to stage, and naming it would + // fail the whole `git add`, publishing nothing of this run: one it never + // tracked, or one an earlier attempt already committed the removal of. + const removed = inCheckout.filter((rel) => !fs.existsSync(path.join(checkout, rel))); + let tracked = new Set<string>(); + if (removed.length > 0) { + try { + tracked = new Set((await createGit(checkout).raw(['--literal-pathspecs', 'ls-files', '-z', '--', ...removed])).split('\0').filter(Boolean)); + } catch (e) { + // Not a checkout git can read, such as an HTTP install's cache: the same + // non-fatal result commitAndPush gives, never a throw after the removal. + return { status: 'failed', reason: failureReason(e) }; + } + } + const files = inCheckout.filter((rel) => !removed.includes(rel) || tracked.has(rel.split(path.sep).join('/'))); + // kind: 'http' has no branch an earlier attempt could have committed to. + if (files.length === 0 && !learningsBranch.enabled(localConfig)) return { status: 'already-present' }; // `commitAndPush`, not `update`: maintenance already wrote into the worktree // before this call, and `update` syncs with origin first, which can carry // those uncommitted files into a rebase or leave them behind. - return learningsBranch.commitAndPush(localConfig, message, ['learnings']); + return learningsBranch.commitAndPush(localConfig, message, files); } /** diff --git a/src/utils/pending-learnings.ts b/src/utils/pending-learnings.ts index 0cdb53862..e87ed457b 100644 --- a/src/utils/pending-learnings.ts +++ b/src/utils/pending-learnings.ts @@ -188,6 +188,32 @@ export async function listPendingForInstall( return locked.status === 'done' ? locked.value : locked; } +/** + * Every queued learning's content, when the queue is still the install's that + * `localConfig` was loaded from, as listPendingForInstall decides it. Read under + * the queue lock, so no learning another install queued after a switch is read + * as this one's. Unreadable entries are left out. + */ +export async function readPendingForInstall( + localConfig: LocalConfig, +): Promise< + | { status: 'read'; queued: Array<{ relPath: string; content: string }> } + | { status: 'busy'; lockPath: string } + | { status: 'changed'; configPath: string; cause: string } +> { + const locked = await withQueueLock(queueHome(localConfig), async () => { + const changed = await installChanged(localConfig); + if (changed) return { status: 'changed' as const, ...changed }; + const queued: Array<{ relPath: string; content: string }> = []; + for (const relPath of await listPendingLearnings(localConfig)) { + const content = await readPendingLearning(localConfig, relPath); + if (content !== null) queued.push({ relPath, content }); + } + return { status: 'read' as const, queued }; + }); + return locked.status === 'done' ? locked.value : locked; +} + /** * Every queued learning, as paths relative to `learnings/`, oldest entries * included. Hidden files and anything that is not Markdown are ignored, so a diff --git a/src/utils/reports-branch.ts b/src/utils/reports-branch.ts index 3a66e8fe7..29257ecf8 100644 --- a/src/utils/reports-branch.ts +++ b/src/utils/reports-branch.ts @@ -84,15 +84,19 @@ export function refreshReportsWorktree( * that copy is another repository's. */ export async function readableReportsWorktree(localConfig: LocalConfig): Promise<string> { - const refreshed = await reportsBranch.refresh(localConfig, { pushIfCreated: false }); + let refreshed = await reportsBranch.refresh(localConfig, { pushIfCreated: false }); + // Failed: try once more, under the lock like every creation, and throw the + // cause if it fails again. An `ensure` here would create the checkout without + // the lock, while a writer that took it meanwhile creates it too (#823 item 15). + if (refreshed.status === 'failed') refreshed = await reportsBranch.refresh(localConfig, { pushIfCreated: false }); switch (refreshed.status) { + case 'done': + return reportsBranch.dir(localConfig); case 'busy': await reportsBranch.checkOwner(localConfig); return reportsBranch.dir(localConfig); - // Failed: ensure tries once more, and throws its cause if it fails again. - case 'done': case 'failed': - return reportsBranch.ensure(localConfig, { pushIfCreated: false }); + throw new Error(refreshed.reason); default: { const unhandled: never = refreshed; throw new Error(`Unhandled refresh result: ${JSON.stringify(unhandled)}`);