From 9f5d4ffa4f42156ef0851db51075dea748e348da Mon Sep 17 00:00:00 2001 From: Saul Moro Date: Wed, 14 Oct 2026 19:53:04 +0200 Subject: [PATCH 1/6] test(session): reproduce a pull and a push that stop on a diverged session index (#1133) --- src/__tests__/session-cmd.test.ts | 21 +- src/__tests__/session-sync-pull.test.ts | 324 ++++++++++++++++++++++++ 2 files changed, 344 insertions(+), 1 deletion(-) create mode 100644 src/__tests__/session-sync-pull.test.ts diff --git a/src/__tests__/session-cmd.test.ts b/src/__tests__/session-cmd.test.ts index a46114e2a..cf66fed67 100644 --- a/src/__tests__/session-cmd.test.ts +++ b/src/__tests__/session-cmd.test.ts @@ -27,6 +27,7 @@ const mocks = vi.hoisted(() => ({ /** 非空时 git commit / git push 抛错,用于验证失败路径 */ commitError: '', pushError: '', + pullError: '', gitCalls: [] as Array<{ args: string[]; cwd?: string }>, adaptersByPlatform: {} as Record, /** migrate.js mock 的 preview 返回值(Error 时抛出);agent-session.js mock 的 copy 返回值 */ @@ -69,6 +70,7 @@ vi.mock('node:child_process', async (importOriginal) => { if (args[0] === 'rev-parse') return 'abc123def456\n'; if (args[0] === 'commit' && mocks.commitError) throw new Error(mocks.commitError); if (args[0] === 'push' && mocks.pushError) throw new Error(mocks.pushError); + if (args.includes('pull') && mocks.pullError) throw new Error(mocks.pullError); return ''; // add / commit / push / pull }, }; @@ -178,6 +180,7 @@ beforeEach(() => { mocks.porcelain = 'M sessions/changed\n'; mocks.commitError = ''; mocks.pushError = ''; + mocks.pullError = ''; process.exitCode = 0; mocks.gitCalls.length = 0; mocks.adaptersByPlatform = {}; @@ -331,7 +334,7 @@ describe('session pull --all', () => { const text = out.join('\n'); expect(text).toContain('Pulled and indexed 2 session(s) across 2 repo(s)'); - expect(mocks.gitCalls.some((c) => c.args[0] === 'pull')).toBe(true); + expect(mocks.gitCalls.some((c) => c.args.includes('pull'))).toBe(true); const restored = JSON.parse(fs.readFileSync(idxPath, 'utf-8')); expect(restored.sessions).toHaveLength(1); @@ -359,6 +362,22 @@ describe('session pull', () => { }); }); +describe('session pull failure', () => { + it("prints git's own reason instead of the command line", async () => { + mocks.pullError = 'Command failed: git pull --no-rebase --ff --no-edit origin main\n' + + 'hint: You have divergent branches.\nfatal: unable to access the remote: Could not resolve host'; + vi.spyOn(process, 'exit').mockImplementation((code) => { + throw new Error(`exit ${code}`); + }); + + await expect(runSession('pull', '--all', '--repo-root', repoRoot)).rejects.toThrow('exit 1'); + + expect(errored.join('\n')).toContain( + `Error: git pull failed in ${repoRoot}: fatal: unable to access the remote: Could not resolve host`, + ); + }); +}); + describe('session search --all', () => { it('searches every repo including _unattributed without double-loading the current one', async () => { seed('github.com/org/alpha', 'alice', 'a-1', 'payment alpha', 'alpha payment retry'); diff --git a/src/__tests__/session-sync-pull.test.ts b/src/__tests__/session-sync-pull.test.ts new file mode 100644 index 000000000..aa8dee65c --- /dev/null +++ b/src/__tests__/session-sync-pull.test.ts @@ -0,0 +1,324 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { execFileSync } from 'node:child_process'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { SyncManager, defaultSyncMeta, encodeRepoIdentity } from '../session-flow/sync.js'; +import type { Session } from '../session-flow/ir.js'; + +const REPO = 'github.com/acme/app'; +const INDEX = `sessions/repos/${encodeRepoIdentity(REPO)}/_index.json`; + +let tmp: string; +const savedEnv = { global: process.env.GIT_CONFIG_GLOBAL, nosystem: process.env.GIT_CONFIG_NOSYSTEM }; + +function git(cwd: string, ...args: string[]): string { + return execFileSync('git', args, { cwd, encoding: 'utf-8', stdio: 'pipe' }).trim(); +} + +function useGitConfig(extra = ''): void { + const file = path.join(tmp, 'gitconfig'); + fs.writeFileSync(file, `[user]\n\tname = tester\n\temail = tester@example.com\n${extra}`); + process.env.GIT_CONFIG_GLOBAL = file; + process.env.GIT_CONFIG_NOSYSTEM = '1'; +} + +function archive(clone: string, sessionId: string, author: string): void { + const session: Session = { + sessionId, + title: `session ${sessionId}`, + cwd: '/proj/app', + platform: 'claude-code', + createdAt: '2026-01-02T03:04:05.000Z', + updatedAt: '2026-01-02T03:05:05.000Z', + messages: [{ role: 'user', content: [{ type: 'text', text: `work on ${sessionId}` }] }], + metadata: {}, + }; + const meta = defaultSyncMeta( + { platform: 'claude-code', author, cwd: '/proj/app', sessionId, repoIdentity: REPO, title: session.title }, + session.createdAt, + ); + const mgr = new SyncManager(clone); + mgr.saveSession(session, meta); + mgr.gitCommit(`sync: ${sessionId}`); +} + +function clone(name: string): string { + const dir = path.join(tmp, name); + git(tmp, 'clone', '-q', path.join(tmp, 'remote.git'), dir); + return dir; +} + +function indexedSessionIds(clone: string): string[] { + const index = JSON.parse(fs.readFileSync(path.join(clone, INDEX), 'utf-8')) as { sessions: { sessionId: string }[] }; + return index.sessions.map((s) => s.sessionId).sort(); +} + +function conflictOnNotes(a: string, b: string): void { + for (const [clone, text] of [[a, 'from a\n'], [b, 'from b\n']]) { + fs.writeFileSync(path.join(clone, 'notes.md'), text); + git(clone, 'add', 'notes.md'); + git(clone, 'commit', '-qm', 'notes'); + } + git(a, 'push', '-q', 'origin', 'main'); +} + +function seedRemote(): { a: string; b: string } { + git(tmp, 'init', '-q', '--bare', '-b', 'main', 'remote.git'); + const seed = clone('seed'); + archive(seed, 's0', 'carol'); + git(seed, 'push', '-q', 'origin', 'main'); + return { a: clone('a'), b: clone('b') }; +} + +function divergeOnIndex(): { a: string; b: string } { + const { a, b } = seedRemote(); + archive(a, 'a1', 'alice'); + git(a, 'push', '-q', 'origin', 'main'); + archive(b, 'b1', 'bob'); + return { a, b }; +} + +beforeEach(() => { + tmp = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'teamai-session-pull-'))); +}); + +afterEach(() => { + process.env.GIT_CONFIG_GLOBAL = savedEnv.global; + process.env.GIT_CONFIG_NOSYSTEM = savedEnv.nosystem; + if (savedEnv.global === undefined) delete process.env.GIT_CONFIG_GLOBAL; + if (savedEnv.nosystem === undefined) delete process.env.GIT_CONFIG_NOSYSTEM; + fs.rmSync(tmp, { recursive: true, force: true }); +}); + +describe('SyncManager.gitPull', () => { + it.each([ + ['no pull config', ''], + ['pull.rebase=false', '[pull]\n\trebase = false\n'], + ['pull.rebase=true', '[pull]\n\trebase = true\n'], + ['pull.ff=only', '[pull]\n\tff = only\n'], + ])('merges a diverged session index and rebuilds it (%s)', (_label, pullConfig) => { + useGitConfig(pullConfig); + const { b } = divergeOnIndex(); + + const result = new SyncManager(b).gitPull(); + + expect(result).toEqual({ status: 'indexes-rebuilt', indexes: [INDEX] }); + expect(indexedSessionIds(b)).toEqual(['a1', 'b1', 's0']); + expect(git(b, 'status', '--porcelain')).toBe(''); + }); + + it('aborts the merge and names the files when another file conflicts too', () => { + useGitConfig(); + const { a, b } = divergeOnIndex(); + conflictOnNotes(a, b); + const headBefore = git(b, 'rev-parse', 'HEAD'); + + const result = new SyncManager(b).gitPull(); + + expect(result).toEqual({ + status: 'stopped', + reason: `Pull stopped on conflicts in notes.md and was aborted. Run \`git pull --no-rebase\` in ${b}, resolve and \`git add\` those files, then run \`teamai session pull\` to rebuild the session indexes and finish the merge.`, + }); + expect(git(b, 'rev-parse', 'HEAD')).toBe(headBefore); + expect(git(b, 'status', '--porcelain')).toBe(''); + }); + + it('points at git commit when only another file conflicts', () => { + useGitConfig(); + const { a, b } = seedRemote(); + conflictOnNotes(a, b); + + const result = new SyncManager(b).gitPull(); + + expect(result).toEqual({ + status: 'stopped', + reason: `Pull stopped on conflicts in notes.md and was aborted. Run \`git pull --no-rebase\` in ${b}, resolve and \`git add\` those files, run \`git commit\`, then retry.`, + }); + expect(git(b, 'status', '--porcelain')).toBe(''); + }); + + it('finishes a merge an earlier pull left stopped on the index', () => { + useGitConfig(); + const { b } = divergeOnIndex(); + expect(() => git(b, '-c', 'pull.rebase=false', 'pull', '-q', 'origin', 'main')).toThrow(); + + const result = new SyncManager(b).gitPull(); + + expect(result).toEqual({ status: 'indexes-rebuilt', indexes: [INDEX] }); + expect(indexedSessionIds(b)).toEqual(['a1', 'b1', 's0']); + expect(git(b, 'status', '--porcelain')).toBe(''); + }); + + it('leaves a merge with other conflicts for the user to finish', () => { + useGitConfig(); + const { a, b } = divergeOnIndex(); + conflictOnNotes(a, b); + expect(() => git(b, '-c', 'pull.rebase=false', 'pull', '-q', 'origin', 'main')).toThrow(); + + const result = new SyncManager(b).gitPull(); + + expect(result).toEqual({ + status: 'stopped', + reason: `A merge is in progress in ${b} with conflicts in notes.md. Resolve and \`git add\` them, or run \`git merge --abort\`, then run \`teamai session pull\`.`, + }); + expect(git(b, 'status', '--porcelain')).toMatch(/^AA notes\.md$/m); + }); + + it('leaves a merge with only another file in conflict for the user to commit', () => { + useGitConfig(); + const { a, b } = seedRemote(); + conflictOnNotes(a, b); + expect(() => git(b, '-c', 'pull.rebase=false', 'pull', '-q', 'origin', 'main')).toThrow(); + + const result = new SyncManager(b).gitPull(); + + expect(result).toEqual({ + status: 'stopped', + reason: `A merge is in progress in ${b} with conflicts in notes.md. Resolve and \`git add\` them and run \`git commit\`, or run \`git merge --abort\`, then retry.`, + }); + expect(git(b, 'status', '--porcelain')).toMatch(/^AA notes\.md$/m); + }); + + it('leaves a merge with nothing unmerged for the user to commit', () => { + useGitConfig(); + const { b } = divergeOnIndex(); + git(b, 'checkout', '-qb', 'feature'); + fs.writeFileSync(path.join(b, 'notes.md'), 'feature\n'); + git(b, 'add', 'notes.md'); + git(b, 'commit', '-qm', 'feature'); + git(b, 'checkout', '-q', 'main'); + git(b, 'merge', '-q', '--no-commit', '--no-ff', 'feature'); + const headBefore = git(b, 'rev-parse', 'HEAD'); + + const result = new SyncManager(b).gitPull(); + + expect(result).toEqual({ + status: 'stopped', + reason: `A merge is in progress in ${b}. Run \`git commit\` or \`git merge --abort\` there, then retry.`, + }); + expect(git(b, 'rev-parse', 'HEAD')).toBe(headBefore); + expect(() => git(b, 'rev-parse', '-q', '--verify', 'MERGE_HEAD')).not.toThrow(); + }); + + it('leaves unmerged files from another git operation alone', () => { + useGitConfig(); + const { b } = divergeOnIndex(); + const index = path.join(b, INDEX); + const committed = fs.readFileSync(index, 'utf-8'); + fs.writeFileSync(index, committed.replace('"version": 1', '"version": 2')); + git(b, 'stash', '-q'); + fs.writeFileSync(index, committed.replace('"version": 1', '"version": 3')); + git(b, 'commit', '-qam', 'local index edit'); + expect(() => git(b, 'stash', 'pop', '-q')).toThrow(); + const headBefore = git(b, 'rev-parse', 'HEAD'); + + const result = new SyncManager(b).gitPull(); + + expect(result).toEqual({ + status: 'stopped', + reason: `Unmerged files in ${b} block the pull: ${INDEX}. Finish or abort the git operation that left them, then retry.`, + }); + expect(git(b, 'rev-parse', 'HEAD')).toBe(headBefore); + expect(git(b, 'status', '--porcelain')).toMatch(/^UU /m); + }); + + it.each([ + ['merge.autostash', '[merge]\n\tautostash = true\n'], + ['pull.autostash', '[pull]\n\tautostash = true\n'], + ])('refuses to merge over local index edits instead of autostashing them (%s)', (_label, autostashConfig) => { + useGitConfig(autostashConfig); + const { b } = divergeOnIndex(); + const index = path.join(b, INDEX); + fs.writeFileSync(index, fs.readFileSync(index, 'utf-8').replace('"version": 1', '"version": 2')); + + expect(() => new SyncManager(b).gitPull()).toThrow(/would be overwritten by merge/); + expect(git(b, 'stash', 'list')).toBe(''); + expect(git(b, 'diff', '--name-only')).toBe(INDEX); + }); + + it('keeps the identity a legacy directory name cannot encode', () => { + useGitConfig(); + const identity = 'github.com/acme/my_app'; + const dir = 'sessions/repos/github.com_acme_my_app'; + const writeLegacy = (clone: string, ids: string[]): void => { + for (const id of ids) { + const meta = defaultSyncMeta( + { platform: 'claude-code', author: 'dana', cwd: '/proj/app', sessionId: id, repoIdentity: identity, title: id }, + '2026-01-02T03:04:05.000Z', + ); + fs.mkdirSync(path.join(clone, dir, 'dana'), { recursive: true }); + fs.writeFileSync(path.join(clone, dir, 'dana', `${id}.meta.json`), JSON.stringify(meta)); + fs.writeFileSync(path.join(clone, dir, 'dana', `${id}.jsonl`), '{}\n'); + } + const sessions = ids.map((sessionId) => ({ sessionId })); + fs.writeFileSync(path.join(clone, dir, '_index.json'), JSON.stringify({ version: 1, repoIdentity: identity, sessions }, null, 2)); + git(clone, 'add', '.'); + git(clone, 'commit', '-qm', ids.join(' ')); + }; + git(tmp, 'init', '-q', '--bare', '-b', 'main', 'remote.git'); + const seed = clone('seed'); + writeLegacy(seed, ['s0']); + git(seed, 'push', '-q', 'origin', 'main'); + const a = clone('a'); + const b = clone('b'); + writeLegacy(a, ['s0', 'a1']); + git(a, 'push', '-q', 'origin', 'main'); + writeLegacy(b, ['s0', 'b1']); + + const result = new SyncManager(b).gitPull(); + + expect(result).toEqual({ status: 'indexes-rebuilt', indexes: [`${dir}/_index.json`] }); + const index = JSON.parse(fs.readFileSync(path.join(b, dir, '_index.json'), 'utf-8')) as { repoIdentity: string }; + expect(index.repoIdentity).toBe(identity); + }); + + it('refuses to pull while a rebase is stopped', () => { + useGitConfig(); + const { b } = divergeOnIndex(); + expect(() => git(b, '-c', 'pull.rebase=true', 'pull', '-q', 'origin', 'main')).toThrow(); + + const result = new SyncManager(b).gitPull(); + + expect(result).toEqual({ + status: 'stopped', + reason: `A rebase is in progress in ${b}. Run \`git rebase --abort\` there, then retry.`, + }); + }); +}); + +describe('SyncManager.gitPush', () => { + it('pulls and retries once when the remote moved on', () => { + useGitConfig(); + const { b } = divergeOnIndex(); + + new SyncManager(b).gitPush(); + + expect(indexedSessionIds(clone('fresh'))).toEqual(['a1', 'b1', 's0']); + }); + + it('stops with the pull reason when the retry hits a real conflict', () => { + useGitConfig(); + const { a, b } = divergeOnIndex(); + conflictOnNotes(a, b); + const headBefore = git(b, 'rev-parse', 'HEAD'); + + expect(() => new SyncManager(b).gitPush()).toThrow(`Pull stopped on conflicts in notes.md and was aborted.`); + expect(git(b, 'rev-parse', 'HEAD')).toBe(headBefore); + expect(git(b, 'status', '--porcelain')).toBe(''); + }); + + it('rethrows a rejection a pull cannot fix, without merging', () => { + useGitConfig(); + git(tmp, 'init', '-q', '--bare', '-b', 'main', 'remote.git'); + const seed = clone('seed'); + archive(seed, 's0', 'carol'); + git(seed, 'push', '-q', 'origin', 'main'); + const hook = path.join(tmp, 'remote.git', 'hooks', 'pre-receive'); + fs.writeFileSync(hook, '#!/bin/sh\nexit 1\n', { mode: 0o755 }); + archive(seed, 's1', 'carol'); + + expect(() => new SyncManager(seed).gitPush()).toThrow(/pre-receive hook declined/); + expect(git(seed, 'rev-list', '--merges', 'HEAD')).toBe(''); + }); +}); From 29aea92c1af1852c8e8bee940aa422d3b9cabda0 Mon Sep 17 00:00:00 2001 From: Saul Moro Date: Wed, 14 Oct 2026 19:53:04 +0200 Subject: [PATCH 2/6] fix(session): merge a diverged session index on pull and retry push once (#1133) gitPull ran a bare `git pull`, so the user's pull.rebase decided the outcome: unset refused any divergence, false stopped mid-merge, true stopped mid-rebase. It now merges explicitly, without rebase or autostash. Conflicts only in `_index.json` are rebuilt from the session files, keeping the identity the index recorded, and the merge is committed. Any other conflict aborts the merge, and an unfinished rebase, merge, or other git operation stops the pull untouched. Both come back as a `stopped` result that names the next step. gitPush pulls and pushes again once when the remote moved ahead, so a second machine's push no longer leaves its commit stranded locally. Git runs untranslated, so errors quote git's own reason line. --- CHANGELOG.md | 1 + src/session-flow/session-cmd.ts | 29 ++++--- src/session-flow/sync.ts | 129 +++++++++++++++++++++++++++++++- 3 files changed, 145 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 99a07b153..330f8f25a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -61,6 +61,7 @@ All notable changes to this project will be documented in this file. See [standa ### 🐛 Bug Fixes +- `teamai session pull` no longer stops when two machines archived sessions for the same repo. It merges whatever the member's `pull.rebase` says, rebuilds a conflicted `_index.json` from the session files and commits the merge. A conflict in any other file aborts the merge and names the files, and an unfinished rebase, merge or other git operation is left as it is, with the step that finishes it. `teamai session push` pulls once and pushes again when the remote moved ahead, instead of keeping the commit local. A failed git step prints git's own reason (for [#1133](https://github.com/Tencent/teamai-cli/issues/1133)). - A project `.codex/hooks.json` a repository commits no longer changes with the platform of the member who pulled last. The PowerShell form the built-in Codex hooks need on Windows (#1074) moves from `command` into Codex' Windows-only `commandWindows`, so `command` is the POSIX `bash -lc "..." || true` on every platform, also on Windows, where it used to carry the machine's Git Bash path. Codex on Windows lists such an entry by its `commandWindows`, so the trust step and `teamai doctor` recognise a written entry under either command; before, the trust step reported the hooks as not loaded and left them untrusted. Team hooks are left as their author wrote them. - `teamai ci extract-mr --mode write` updates the teamwiki through `teamai import`: under the same `____` slug as `import --from-repo` and `--from-mr` (it used the bare repo name, so one repo had two codebases), incrementally from the baseline published with the codebase, also for a merge that only deletes source files, rebuilding `router.md` / `index.md`, and without extracting into, then deleting, a `teamwiki/` in the checkout. The GitHub Actions template checks out the merged source, which its write job never did. - An incremental import whose commit changes no source file no longer deletes the codebase's published evidence pages. Each codebase keeps its incremental baseline (facts and interface caches) in `evidence/code//.indices/`, so importing another repo into the same teamwiki no longer forces the next import of this one to run in full. diff --git a/src/session-flow/session-cmd.ts b/src/session-flow/session-cmd.ts index fa02cef89..1c8c2f177 100644 --- a/src/session-flow/session-cmd.ts +++ b/src/session-flow/session-cmd.ts @@ -305,18 +305,20 @@ function pushToRemote(syncMgr: SyncManager): boolean { syncMgr.gitPush(); return true; } catch (err) { - // The first line "Command failed: git push origin" carries no information; - // git's fatal line is the actual reason - const msg = err instanceof Error ? err.message : String(err); - const fatal = msg.split('\n').find((l) => /^(fatal|error):/i.test(l.trim())); - const reason = safeText(fatal?.trim() ?? msg.split('\n')[0]); // The local commit is kept, so nothing is lost -- but this is still a // failure and must not be printed as "✓ Pushed". - console.error(` ✗ Remote push failed (local commit kept): ${reason}`); + console.error(` ✗ Remote push failed (local commit kept): ${gitFailureReason(err)}`); return false; } } +/** The first line ("Command failed: git ...") only repeats the command; git's fatal/error line is the actual reason. */ +function gitFailureReason(err: unknown): string { + const msg = err instanceof Error ? err.message : String(err); + const fatal = msg.split('\n').find((l) => /^(fatal|error):/i.test(l.trim())); + return safeText(fatal?.trim() ?? msg.split('\n')[0]); +} + /** * Wrap gitCommit / gitPull: a git-layer failure (not a git directory, no * remote, index.lock race, and so on) prints a single English error line and @@ -327,8 +329,7 @@ function runGitStep(step: () => T, repoRoot: string, what: string): T { try { return step(); } catch (err) { - const reason = err instanceof Error ? err.message.split('\n')[0] : String(err); - console.error(`Error: ${what} failed in ${repoRoot}: ${reason}`); + console.error(`Error: ${what} failed in ${repoRoot}: ${gitFailureReason(err)}`); console.error(`Check that ${repoRoot} is a git repository with a configured remote, then retry.`); process.exit(1); } @@ -1098,10 +1099,14 @@ export function registerSessionFlowCommands(sessionCmd: Command): void { return; } // [fixed] a gitPull failure (no remote / repoRoot does not exist) used to crash with a raw stack trace - runGitStep(() => { - syncMgr.gitPull(); - return null; - }, repoRoot, 'git pull'); + const pulled = runGitStep(() => syncMgr.gitPull(), repoRoot, 'git pull'); + if (pulled.status === 'stopped') { + console.error(`Error: ${pulled.reason}`); + process.exit(1); + } + if (pulled.status === 'indexes-rebuilt') { + console.log(` ✓ Merged remote sessions and rebuilt ${pulled.indexes.length} conflicted session index(es)`); + } if (opts.all) { // P2: rebuild by directory (including repos whose index is missing or corrupt — those are exactly the ones that most need repair), // rather than only rebuilding the ones whose identity can be recovered from _index.json. diff --git a/src/session-flow/sync.ts b/src/session-flow/sync.ts index 2fc8f455d..457f7630c 100644 --- a/src/session-flow/sync.ts +++ b/src/session-flow/sync.ts @@ -316,6 +316,18 @@ export type GitCommitResult = | { status: 'no-changes' } | { status: 'failed'; reason: string }; +/** The outcomes of `gitPull`. `stopped` merged nothing, so the checkout is as it was before the pull. */ +export type PullResult = + | { status: 'pulled' } + | { status: 'indexes-rebuilt'; indexes: string[] } + | { status: 'stopped'; reason: string }; + +function isSessionIndexPath(relPath: string): boolean { + const parts = relPath.split('/'); + if (parts[0] !== 'sessions' || !isIndexFileName(parts[parts.length - 1])) return false; + return (parts.length === 3 && parts[1] === '_unattributed') || (parts.length === 4 && parts[1] === 'repos'); +} + // --------------------------------------------------------------------------- // SyncManager // --------------------------------------------------------------------------- @@ -1049,6 +1061,8 @@ export class SyncManager { encoding: 'utf-8', timeout: 30_000, stdio: ['pipe', 'pipe', 'pipe'], + // Untranslated, so the CLI's English output can quote git's reason. + env: { ...process.env, LANGUAGE: 'en' }, }).trim(); } catch (e) { if (check) throw e; @@ -1093,15 +1107,126 @@ export class SyncManager { } // Errors must reach the caller: swallowing them here let `push` print // "✓ Pushed" after a rejected or unreachable remote. + try { + this.runGit(['push', remote, ref]); + return; + } catch (err) { + if (!this.isBehindRemote(remote, ref)) throw err; + } + const pulled = this.gitPull(remote, ref); + if (pulled.status === 'stopped') throw new Error(pulled.reason); this.runGit(['push', remote, ref]); } - gitPull(remote = 'origin', branch?: string): void { + /** Asked of git: a push fails for many reasons, and a pull fixes only this one. */ + private isBehindRemote(remote: string, ref: string): boolean { + try { + this.runGit(['fetch', remote, ref]); + } catch { + return false; + } + try { + this.runGit(['merge-base', '--is-ancestor', 'FETCH_HEAD', 'HEAD']); + return false; + } catch { + return true; + } + } + + /** + * Pull and merge. Two machines that archive sessions for the same repo both append to its `_index.json`, + * so their pulls conflict there. The index is derived from the session files, so those conflicts are + * resolved by a rebuild. A conflict in any other file aborts the merge and is returned to the caller. + */ + gitPull(remote = 'origin', branch?: string): PullResult { + if (this.gitPathExists('rebase-merge') || this.gitPathExists('rebase-apply')) { + return { status: 'stopped', reason: `A rebase is in progress in ${this.repoRoot}. Run \`git rebase --abort\` there, then retry.` }; + } + const unmerged = this.unmergedPaths(); + // An older teamai or a manual `git pull` can leave the checkout mid-merge on an index conflict; finish that merge the same way. + if (this.gitPathExists('MERGE_HEAD')) { + if (unmerged.length === 0) { + return { + status: 'stopped', + reason: `A merge is in progress in ${this.repoRoot}. Run \`git commit\` or \`git merge --abort\` there, then retry.`, + }; + } + const others = unmerged.filter((f) => !isSessionIndexPath(f)); + if (others.length > 0) { + // Without an index left to rebuild, teamai has nothing to finish once the user adds the files. + const next = + others.length < unmerged.length + ? 'Resolve and `git add` them, or run `git merge --abort`, then run `teamai session pull`' + : 'Resolve and `git add` them and run `git commit`, or run `git merge --abort`, then retry'; + return { status: 'stopped', reason: `A merge is in progress in ${this.repoRoot} with conflicts in ${others.join(', ')}. ${next}.` }; + } + return this.commitRebuiltIndexes(unmerged); + } + if (unmerged.length > 0) { + return { + status: 'stopped', + reason: `Unmerged files in ${this.repoRoot} block the pull: ${unmerged.join(', ')}. Finish or abort the git operation that left them, then retry.`, + }; + } const ref = branch?.trim() || this.runGit(['rev-parse', '--abbrev-ref', 'HEAD']); if (!ref || ref === 'HEAD') { throw new Error('Refusing to pull without an explicit branch refspec.'); } - this.runGit(['pull', remote, ref]); + // Explicit flags: with pull.rebase unset git refuses any divergence, with it set a conflict stops + // mid-rebase on a detached HEAD, and an autostash reapplied after the merge commit can conflict unseen. + // The autostash is turned off through config (both keys) because `git pull --no-autostash` needs git 2.27. + try { + this.runGit(['-c', 'merge.autostash=false', '-c', 'pull.autostash=false', 'pull', '--no-rebase', '--ff', '--no-edit', remote, ref]); + return { status: 'pulled' }; + } catch (err) { + const files = this.unmergedPaths(); + if (files.length === 0) throw err; + const others = files.filter((f) => !isSessionIndexPath(f)); + if (others.length === 0) return this.commitRebuiltIndexes(files); + this.runGit(['merge', '--abort']); + const next = + others.length < files.length + ? 'then run `teamai session pull` to rebuild the session indexes and finish the merge' + : 'run `git commit`, then retry'; + return { + status: 'stopped', + reason: `Pull stopped on conflicts in ${others.join(', ')} and was aborted. Run \`git pull --no-rebase\` in ${this.repoRoot}, resolve and \`git add\` those files, ${next}.`, + }; + } + } + + private commitRebuiltIndexes(indexes: string[]): Extract { + for (const index of indexes) { + this.rebuildIndex(this.conflictedIndexIdentity(index), path.join(this.repoRoot, path.dirname(index))); + this.runGit(['add', '--', index]); + } + this.runGit(['commit', '--no-edit']); + return { status: 'indexes-rebuilt', indexes }; + } + + /** The identity either side recorded; a legacy `_`-folded directory name cannot be decoded back to it. */ + private conflictedIndexIdentity(index: string): string | null { + for (const stage of [2, 3]) { + try { + return (JSON.parse(this.runGit(['show', `:${stage}:./${index}`])) as RepoIndex).repoIdentity; + } catch { + // that side deleted the index or left it unparsable + } + } + const parts = index.split('/'); + return parts[1] === '_unattributed' ? null : decodeLegacyRepoDirName(parts[2]); + } + + private gitPathExists(name: string): boolean { + return fs.existsSync(path.resolve(this.repoRoot, this.runGit(['rev-parse', '--git-path', name]))); + } + + /** Unmerged paths relative to repoRoot (git reports them from the top of the work tree). */ + private unmergedPaths(): string[] { + const files = this.runGit(['diff', '--name-only', '-z', '--diff-filter=U']).split('\0').filter(Boolean); + if (files.length === 0) return []; + const prefix = this.runGit(['rev-parse', '--show-prefix']); + return files.map((f) => (f.startsWith(prefix) ? f.slice(prefix.length) : path.posix.relative(prefix, f))); } getSyncStatus(): { uncommitted: number; ahead: number; behind: number } { From a5ea147ba8e63e67acbeb5f58f535cc2a6b22a34 Mon Sep 17 00:00:00 2001 From: Saul Moro Date: Wed, 14 Oct 2026 23:02:12 +0200 Subject: [PATCH 3/6] fix(session): rebuild a merged session index under the index lock (#1133) A pull that stops on conflicted session indexes now returns them instead of rebuilding them. session pull and the push retry rebuild them and commit the merge inside withSessionIndexLock, while the network pull and push stay outside it. When the lock stays busy the merge is left in progress and the member is told to rerun `teamai session pull`, which finishes it. --- docs/dev/agent-sessions.md | 2 +- src/__tests__/session-sync-pull.test.ts | 124 +++++++++++++++++++++--- src/session-flow/session-cmd.ts | 30 +++++- src/session-flow/sync.ts | 28 ++++-- 4 files changed, 156 insertions(+), 28 deletions(-) diff --git a/docs/dev/agent-sessions.md b/docs/dev/agent-sessions.md index 939183d44..7fd981162 100644 --- a/docs/dev/agent-sessions.md +++ b/docs/dev/agent-sessions.md @@ -14,7 +14,7 @@ Each rule below spans more than one file. The rule names the code that owns it, - **List through `AgentSession.list`.** A caller that wants a member's sessions (`session migrate`, `session push`, a scorer, the dashboard) calls `AgentSession.list({ platforms?, cwd?, updatedSince? })` in `agent-session.ts`, never an adapter's `listConversations`. Each listed session is a ref that `find` reads as is. A session whose first lines do not parse is still listed, under a placeholder title, so `find` reports it `unreadable`, not `not_found`. A tool that cannot be listed is one failure value (`no_adapter`, `tool_unavailable`, or `list_failed`) next to the other tools' sessions. OpenCode's CLI lists one project at a time, and inside a git repo that project is the whole repo, so its adapter keeps only the sessions of the workspace asked for. Without `cwd`, OpenCode is `list_failed`, rather than returning the workspace the process runs in as if it were all of them. Its listing has no message count or size, so those fields are absent rather than 0. - **Stores follow the recorded tool root.** Claude Code and Codex keep their sessions under the root the member recorded with `teamai init` (`toolRoots`), resolved by `resolveMemberToolRoots` and `resolveToolRootDir`, never from `CLAUDE_CONFIG_DIR` or `CODEX_HOME` at run time. `AgentSession` and the `session` commands that build an adapter themselves (`resume`, `rollback`, `platforms`) resolve the roots with `recordedToolRoots` in `agent-session.ts` for the workspace at hand and pass them through the adapter registry (`adapters/index.ts`) to `getClaudeCodeProjectsDir` and `getCodexSessionsDir` in `fs.ts`. A run that reads or copies many sessions passes one `toolRootsLookup()`, so each workspace is resolved once. With nothing recorded they stay `~/.claude/projects` and `~/.codex/sessions`. The variants (`claude-internal`, `codex-internal`, ...) keep their fixed paths. - **The driver picks how a session continues.** The driver that runs a workflow step decides: the same tool on the same machine resumes the session by id; a driver with its own way to move a session uses it; otherwise the local driver copies it with `AgentSession.copy`; failing those, the step starts a fresh session. The workflow engine never copies a session. A step's report carries the tool and that tool's native session id. -- **Session index lock.** Every write to a team repo archive (`saveSession`: dedup lookup, name choice, file writes, `_index.json` upsert) and every index rebuild (`rebuildIndex`, `rebuildAllIndexes`) runs inside `withSessionIndexLock(repoRoot, fn)`. The async command callers take it; `SyncManager` stays synchronous and never takes it, so it cannot re-enter. The lock file is `~/.teamai/locks/sessions-.lock`, keyed by the real path of the team repo's `sessions/`. It stays out of the team repo because there the archive commit could stage it and `pull --all`'s directory renames could move it while held. It covers one machine only; two machines writing `_index.json` is #1133. +- **Session index lock.** Every write to a team repo archive (`saveSession`: dedup lookup, name choice, file writes, `_index.json` upsert) and every index rebuild (`rebuildIndex`, `rebuildAllIndexes`) runs inside `withSessionIndexLock(repoRoot, fn)`. The async command callers take it; `SyncManager` stays synchronous and never takes it, so it cannot re-enter. The lock file is `~/.teamai/locks/sessions-.lock`, keyed by the real path of the team repo's `sessions/`. It stays out of the team repo because there the archive commit could stage it and `pull --all`'s directory renames could move it while held. It covers one machine only. Two machines that both wrote `_index.json` diverge there: `SyncManager.gitPull` (also the retry inside `gitPush`) merges and returns the conflicted indexes, and the command caller rebuilds them and commits the merge (`commitRebuiltIndexes`) under the lock, with the network pull and push outside it. When the lock stays busy the merge is left in progress, and the next `teamai session pull` finishes it. ## Tests diff --git a/src/__tests__/session-sync-pull.test.ts b/src/__tests__/session-sync-pull.test.ts index aa8dee65c..3a1477b62 100644 --- a/src/__tests__/session-sync-pull.test.ts +++ b/src/__tests__/session-sync-pull.test.ts @@ -1,16 +1,34 @@ -import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import { execFileSync } from 'node:child_process'; import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; -import { SyncManager, defaultSyncMeta, encodeRepoIdentity } from '../session-flow/sync.js'; +import { Command } from 'commander'; +import { SyncManager, defaultSyncMeta, encodeRepoIdentity, type PullResult } from '../session-flow/sync.js'; +import { registerSessionFlowCommands } from '../session-flow/session-cmd.js'; import type { Session } from '../session-flow/ir.js'; +const lock = vi.hoisted(() => ({ + /** When set, the session archive lock reports this path as held by another command. */ + heldBy: '', +})); + +vi.mock('../session-flow/sync.js', async (importOriginal) => { + const actual = (await importOriginal()) as typeof import('../session-flow/sync.js'); + return { + ...actual, + withSessionIndexLock: (async (repo, fn) => + lock.heldBy + ? { status: 'busy', lockPath: lock.heldBy } + : actual.withSessionIndexLock(repo, fn)) as typeof actual.withSessionIndexLock, + }; +}); + const REPO = 'github.com/acme/app'; const INDEX = `sessions/repos/${encodeRepoIdentity(REPO)}/_index.json`; let tmp: string; -const savedEnv = { global: process.env.GIT_CONFIG_GLOBAL, nosystem: process.env.GIT_CONFIG_NOSYSTEM }; +const savedEnv = { global: process.env.GIT_CONFIG_GLOBAL, nosystem: process.env.GIT_CONFIG_NOSYSTEM, home: process.env.HOME }; function git(cwd: string, ...args: string[]): string { return execFileSync('git', args, { cwd, encoding: 'utf-8', stdio: 'pipe' }).trim(); @@ -49,11 +67,60 @@ function clone(name: string): string { return dir; } -function indexedSessionIds(clone: string): string[] { - const index = JSON.parse(fs.readFileSync(path.join(clone, INDEX), 'utf-8')) as { sessions: { sessionId: string }[] }; +function sessionIdsOf(indexJson: string): string[] { + const index = JSON.parse(indexJson) as { sessions: { sessionId: string }[] }; return index.sessions.map((s) => s.sessionId).sort(); } +function indexedSessionIds(clone: string): string[] { + return sessionIdsOf(fs.readFileSync(path.join(clone, INDEX), 'utf-8')); +} + +/** Pull, and finish a merge stopped on the session indexes the way the session commands do. */ +function pullAndRebuild(clone: string): PullResult { + const mgr = new SyncManager(clone); + const result = mgr.gitPull(); + if (result.status === 'indexes-conflicted') mgr.commitRebuiltIndexes(result.indexes); + return result; +} + +class ExitCalled extends Error { + constructor(readonly code: number) { + super(`exit ${code}`); + } +} + +/** Runs `teamai session pull --all` against the team repo checkout. */ +async function sessionPull(teamRepo: string): Promise<{ code: number; stdout: string; stderr: string }> { + const stdout: string[] = []; + const stderr: string[] = []; + vi.spyOn(console, 'log').mockImplementation((...a: unknown[]) => { + stdout.push(a.map(String).join(' ')); + }); + vi.spyOn(console, 'error').mockImplementation((...a: unknown[]) => { + stderr.push(a.map(String).join(' ')); + }); + vi.spyOn(process, 'exit').mockImplementation((code) => { + throw new ExitCalled(Number(code ?? 0)); + }); + process.exitCode = 0; + let code: number; + try { + const program = new Command(); + program.option('--dry-run', 'preview'); + registerSessionFlowCommands(program.command('session')); + await program.parseAsync(['node', 'teamai', 'session', 'pull', '--all', '--repo-root', teamRepo]); + code = Number(process.exitCode ?? 0); + } catch (err) { + if (!(err instanceof ExitCalled)) throw err; + code = err.code; + } finally { + vi.restoreAllMocks(); + process.exitCode = 0; + } + return { code, stdout: stdout.join('\n'), stderr: stderr.join('\n') }; +} + function conflictOnNotes(a: string, b: string): void { for (const [clone, text] of [[a, 'from a\n'], [b, 'from b\n']]) { fs.writeFileSync(path.join(clone, 'notes.md'), text); @@ -81,9 +148,13 @@ function divergeOnIndex(): { a: string; b: string } { beforeEach(() => { tmp = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'teamai-session-pull-'))); + process.env.HOME = path.join(tmp, 'home'); + lock.heldBy = ''; }); afterEach(() => { + process.env.HOME = savedEnv.home; + if (savedEnv.home === undefined) delete process.env.HOME; process.env.GIT_CONFIG_GLOBAL = savedEnv.global; process.env.GIT_CONFIG_NOSYSTEM = savedEnv.nosystem; if (savedEnv.global === undefined) delete process.env.GIT_CONFIG_GLOBAL; @@ -101,9 +172,9 @@ describe('SyncManager.gitPull', () => { useGitConfig(pullConfig); const { b } = divergeOnIndex(); - const result = new SyncManager(b).gitPull(); + const result = pullAndRebuild(b); - expect(result).toEqual({ status: 'indexes-rebuilt', indexes: [INDEX] }); + expect(result).toEqual({ status: 'indexes-conflicted', indexes: [INDEX] }); expect(indexedSessionIds(b)).toEqual(['a1', 'b1', 's0']); expect(git(b, 'status', '--porcelain')).toBe(''); }); @@ -143,9 +214,9 @@ describe('SyncManager.gitPull', () => { const { b } = divergeOnIndex(); expect(() => git(b, '-c', 'pull.rebase=false', 'pull', '-q', 'origin', 'main')).toThrow(); - const result = new SyncManager(b).gitPull(); + const result = pullAndRebuild(b); - expect(result).toEqual({ status: 'indexes-rebuilt', indexes: [INDEX] }); + expect(result).toEqual({ status: 'indexes-conflicted', indexes: [INDEX] }); expect(indexedSessionIds(b)).toEqual(['a1', 'b1', 's0']); expect(git(b, 'status', '--porcelain')).toBe(''); }); @@ -266,9 +337,9 @@ describe('SyncManager.gitPull', () => { git(a, 'push', '-q', 'origin', 'main'); writeLegacy(b, ['s0', 'b1']); - const result = new SyncManager(b).gitPull(); + const result = pullAndRebuild(b); - expect(result).toEqual({ status: 'indexes-rebuilt', indexes: [`${dir}/_index.json`] }); + expect(result).toEqual({ status: 'indexes-conflicted', indexes: [`${dir}/_index.json`] }); const index = JSON.parse(fs.readFileSync(path.join(b, dir, '_index.json'), 'utf-8')) as { repoIdentity: string }; expect(index.repoIdentity).toBe(identity); }); @@ -287,13 +358,40 @@ describe('SyncManager.gitPull', () => { }); }); +describe('session pull', () => { + it('leaves a merge stopped on the session index for a rerun while another command holds the archive lock', async () => { + useGitConfig(); + const { b } = divergeOnIndex(); + lock.heldBy = path.join(tmp, 'home', '.teamai', 'locks', 'sessions-held.lock'); + + const busy = await sessionPull(b); + + expect(git(b, 'status', '--porcelain')).toContain(`UU ${INDEX}`); + expect(busy.code).toBe(1); + expect(busy.stderr).toContain(`another teamai command is writing the session archive in ${b} (lock: ${lock.heldBy}).`); + expect(busy.stderr).toContain('run `teamai session pull` once it has finished'); + + lock.heldBy = ''; + const rerun = await sessionPull(b); + + expect(rerun.code).toBe(0); + expect(rerun.stdout).toContain('Merged remote sessions and rebuilt 1 conflicted session index(es)'); + expect(() => git(b, 'rev-parse', '-q', '--verify', 'MERGE_HEAD')).toThrow(); + expect(sessionIdsOf(git(b, 'show', `HEAD:${INDEX}`))).toEqual(['a1', 'b1', 's0']); + }); +}); + describe('SyncManager.gitPush', () => { - it('pulls and retries once when the remote moved on', () => { + it('pulls once when the remote moved on and leaves a merge stopped on the index to the caller', () => { useGitConfig(); const { b } = divergeOnIndex(); + const mgr = new SyncManager(b); - new SyncManager(b).gitPush(); + expect(mgr.gitPush()).toEqual({ status: 'indexes-conflicted', indexes: [INDEX] }); + expect(git(b, 'status', '--porcelain')).toContain(`UU ${INDEX}`); + mgr.commitRebuiltIndexes([INDEX]); + expect(mgr.gitPush()).toEqual({ status: 'pushed' }); expect(indexedSessionIds(clone('fresh'))).toEqual(['a1', 'b1', 's0']); }); diff --git a/src/session-flow/session-cmd.ts b/src/session-flow/session-cmd.ts index 1c8c2f177..f7bc072c0 100644 --- a/src/session-flow/session-cmd.ts +++ b/src/session-flow/session-cmd.ts @@ -300,9 +300,16 @@ async function resolveRepoRoot( * up with a stack trace would disguise a successful archive as a total * failure, and running it again would create duplicate commits. */ -function pushToRemote(syncMgr: SyncManager): boolean { +async function pushToRemote(syncMgr: SyncManager, repoRoot: string): Promise { try { - syncMgr.gitPush(); + const pushed = syncMgr.gitPush(); + if (pushed.status === 'indexes-conflicted') { + const merged = await withSessionIndexLock(repoRoot, async () => syncMgr.commitRebuiltIndexes(pushed.indexes)); + if (merged.status === 'busy') throw new Error(mergeBusyMessage(repoRoot, merged.lockPath)); + if (syncMgr.gitPush().status !== 'pushed') { + throw new Error(`The remote moved on again and its merge stopped on the session indexes; run \`teamai session pull\` to finish it.`); + } + } return true; } catch (err) { // The local commit is kept, so nothing is lost -- but this is still a @@ -339,6 +346,11 @@ function archiveBusyMessage(repoRoot: string, lockPath: string): string { return `another teamai command is writing the session archive in ${repoRoot} (lock: ${lockPath}).`; } +/** A pull merged the remote, but the session indexes it left conflicted wait for the lock another command holds. */ +function mergeBusyMessage(repoRoot: string, lockPath: string): string { + return `${archiveBusyMessage(repoRoot, lockPath)} The pull is left mid-merge on the session indexes; run \`teamai session pull\` once it has finished.`; +} + /** Another command held the session archive lock for the whole wait: this write did not happen, so fail the run. */ function reportArchiveBusy(repoRoot: string, lockPath: string): void { console.error(`Error: ${archiveBusyMessage(repoRoot, lockPath)}`); @@ -854,7 +866,7 @@ export function registerSessionFlowCommands(sessionCmd: Command): void { const commitArchive = () => syncMgr.gitCommit(`sync: migrate ${saved} session(s) ${source}→${target}`); const commit = json ? commitArchive() : runGitStep(commitArchive, repoRoot, 'git commit'); if (commit.status === 'committed') { - const pushed = pushToRemote(syncMgr); + const pushed = await pushToRemote(syncMgr, repoRoot); if (pushed) { console.log(`\n ✓ Pushed ${saved} session(s) to team repo`); } else { @@ -1061,7 +1073,7 @@ export function registerSessionFlowCommands(sessionCmd: Command): void { 'git commit', ); if (commit.status === 'committed') { - const pushed = pushToRemote(syncMgr); + const pushed = await pushToRemote(syncMgr, repoRoot); if (pushed) { console.log(`\n ✓ Pushed ${saved} session(s) from ${source}`); } else { @@ -1104,7 +1116,15 @@ export function registerSessionFlowCommands(sessionCmd: Command): void { console.error(`Error: ${pulled.reason}`); process.exit(1); } - if (pulled.status === 'indexes-rebuilt') { + if (pulled.status === 'indexes-conflicted') { + // The rebuild takes the lock and the network pull above does not: a lock held across it would make other saves wait and fail busy. + const merged = await withSessionIndexLock(repoRoot, async () => + runGitStep(() => syncMgr.commitRebuiltIndexes(pulled.indexes), repoRoot, 'git pull'), + ); + if (merged.status === 'busy') { + console.error(`Error: ${mergeBusyMessage(repoRoot, merged.lockPath)}`); + process.exit(1); + } console.log(` ✓ Merged remote sessions and rebuilt ${pulled.indexes.length} conflicted session index(es)`); } if (opts.all) { diff --git a/src/session-flow/sync.ts b/src/session-flow/sync.ts index 457f7630c..1428e27f4 100644 --- a/src/session-flow/sync.ts +++ b/src/session-flow/sync.ts @@ -316,12 +316,19 @@ export type GitCommitResult = | { status: 'no-changes' } | { status: 'failed'; reason: string }; -/** The outcomes of `gitPull`. `stopped` merged nothing, so the checkout is as it was before the pull. */ +/** + * The outcomes of `gitPull`. `stopped` merged nothing, so the checkout is as it was before the pull. + * `indexes-conflicted` leaves the merge in progress with only these session indexes unmerged: the caller + * finishes it with `commitRebuiltIndexes` under `withSessionIndexLock`. + */ export type PullResult = | { status: 'pulled' } - | { status: 'indexes-rebuilt'; indexes: string[] } + | { status: 'indexes-conflicted'; indexes: string[] } | { status: 'stopped'; reason: string }; +/** The outcomes of `gitPush`. `indexes-conflicted` is the retry's pull, left for the caller as in `PullResult`. */ +export type PushResult = { status: 'pushed' } | { status: 'indexes-conflicted'; indexes: string[] }; + function isSessionIndexPath(relPath: string): boolean { const parts = relPath.split('/'); if (parts[0] !== 'sessions' || !isIndexFileName(parts[parts.length - 1])) return false; @@ -1100,7 +1107,7 @@ export class SyncManager { return { status: 'committed', commit: this.runGit(['rev-parse', 'HEAD']) }; } - gitPush(remote = 'origin', branch?: string): void { + gitPush(remote = 'origin', branch?: string): PushResult { const ref = branch?.trim() || this.runGit(['rev-parse', '--abbrev-ref', 'HEAD']); if (!ref || ref === 'HEAD') { throw new Error('Refusing to push without an explicit branch refspec.'); @@ -1109,13 +1116,15 @@ export class SyncManager { // "✓ Pushed" after a rejected or unreachable remote. try { this.runGit(['push', remote, ref]); - return; + return { status: 'pushed' }; } catch (err) { if (!this.isBehindRemote(remote, ref)) throw err; } const pulled = this.gitPull(remote, ref); if (pulled.status === 'stopped') throw new Error(pulled.reason); + if (pulled.status === 'indexes-conflicted') return pulled; this.runGit(['push', remote, ref]); + return { status: 'pushed' }; } /** Asked of git: a push fails for many reasons, and a pull fixes only this one. */ @@ -1136,7 +1145,8 @@ export class SyncManager { /** * Pull and merge. Two machines that archive sessions for the same repo both append to its `_index.json`, * so their pulls conflict there. The index is derived from the session files, so those conflicts are - * resolved by a rebuild. A conflict in any other file aborts the merge and is returned to the caller. + * resolved by a rebuild, which this leaves to the caller: the rebuild takes the session index lock, and + * the network pull must not. A conflict in any other file aborts the merge and is returned to the caller. */ gitPull(remote = 'origin', branch?: string): PullResult { if (this.gitPathExists('rebase-merge') || this.gitPathExists('rebase-apply')) { @@ -1160,7 +1170,7 @@ export class SyncManager { : 'Resolve and `git add` them and run `git commit`, or run `git merge --abort`, then retry'; return { status: 'stopped', reason: `A merge is in progress in ${this.repoRoot} with conflicts in ${others.join(', ')}. ${next}.` }; } - return this.commitRebuiltIndexes(unmerged); + return { status: 'indexes-conflicted', indexes: unmerged }; } if (unmerged.length > 0) { return { @@ -1182,7 +1192,7 @@ export class SyncManager { const files = this.unmergedPaths(); if (files.length === 0) throw err; const others = files.filter((f) => !isSessionIndexPath(f)); - if (others.length === 0) return this.commitRebuiltIndexes(files); + if (others.length === 0) return { status: 'indexes-conflicted', indexes: files }; this.runGit(['merge', '--abort']); const next = others.length < files.length @@ -1195,13 +1205,13 @@ export class SyncManager { } } - private commitRebuiltIndexes(indexes: string[]): Extract { + /** Finish a merge `gitPull` left on `indexes-conflicted`: rebuild those indexes from the session files and commit. */ + commitRebuiltIndexes(indexes: string[]): void { for (const index of indexes) { this.rebuildIndex(this.conflictedIndexIdentity(index), path.join(this.repoRoot, path.dirname(index))); this.runGit(['add', '--', index]); } this.runGit(['commit', '--no-edit']); - return { status: 'indexes-rebuilt', indexes }; } /** The identity either side recorded; a legacy `_`-folded directory name cannot be decoded back to it. */ From 7d562996e41fe96f7d0cf4eadf45fa856fea8c45 Mon Sep 17 00:00:00 2001 From: Saul Moro Date: Thu, 15 Oct 2026 00:49:56 +0200 Subject: [PATCH 4/6] test(session): push --all skips an unreadable session and still merges a diverged index (#1133) --- src/__tests__/session-sync-pull.test.ts | 42 ++++++++++++++++++++++--- 1 file changed, 37 insertions(+), 5 deletions(-) diff --git a/src/__tests__/session-sync-pull.test.ts b/src/__tests__/session-sync-pull.test.ts index 3a1477b62..e6d4f41c1 100644 --- a/src/__tests__/session-sync-pull.test.ts +++ b/src/__tests__/session-sync-pull.test.ts @@ -90,8 +90,8 @@ class ExitCalled extends Error { } } -/** Runs `teamai session pull --all` against the team repo checkout. */ -async function sessionPull(teamRepo: string): Promise<{ code: number; stdout: string; stderr: string }> { +/** Runs `teamai session `. */ +async function session(...args: string[]): Promise<{ code: number; stdout: string; stderr: string }> { const stdout: string[] = []; const stderr: string[] = []; vi.spyOn(console, 'log').mockImplementation((...a: unknown[]) => { @@ -109,7 +109,7 @@ async function sessionPull(teamRepo: string): Promise<{ code: number; stdout: st const program = new Command(); program.option('--dry-run', 'preview'); registerSessionFlowCommands(program.command('session')); - await program.parseAsync(['node', 'teamai', 'session', 'pull', '--all', '--repo-root', teamRepo]); + await program.parseAsync(['node', 'teamai', 'session', ...args]); code = Number(process.exitCode ?? 0); } catch (err) { if (!(err instanceof ExitCalled)) throw err; @@ -364,7 +364,7 @@ describe('session pull', () => { const { b } = divergeOnIndex(); lock.heldBy = path.join(tmp, 'home', '.teamai', 'locks', 'sessions-held.lock'); - const busy = await sessionPull(b); + const busy = await session('pull', '--all', '--repo-root', b); expect(git(b, 'status', '--porcelain')).toContain(`UU ${INDEX}`); expect(busy.code).toBe(1); @@ -372,7 +372,7 @@ describe('session pull', () => { expect(busy.stderr).toContain('run `teamai session pull` once it has finished'); lock.heldBy = ''; - const rerun = await sessionPull(b); + const rerun = await session('pull', '--all', '--repo-root', b); expect(rerun.code).toBe(0); expect(rerun.stdout).toContain('Merged remote sessions and rebuilt 1 conflicted session index(es)'); @@ -381,6 +381,38 @@ describe('session pull', () => { }); }); +describe('session push --all', () => { + it('skips an unreadable session, still merges a diverged index and pushes the rest, and exits 1', async () => { + useGitConfig(); + const { b } = divergeOnIndex(); + const projects = path.join(tmp, 'home', '.claude', 'projects', '-proj-app'); + fs.mkdirSync(projects, { recursive: true }); + const record = (text: string) => + JSON.stringify({ + type: 'user', + message: { role: 'user', content: [{ type: 'text', text }] }, + cwd: '/proj/app', + timestamp: '2026-01-02T00:00:00.000Z', + uuid: 'u1', + parentUuid: null, + isSidechain: false, + }) + '\n'; + fs.writeFileSync(path.join(projects, '11111111-1111-4111-8111-111111111111.jsonl'), record('first')); + fs.writeFileSync(path.join(projects, '22222222-2222-4222-8222-222222222222.jsonl'), '{"truncated\n'); + fs.writeFileSync(path.join(projects, '33333333-3333-4333-8333-333333333333.jsonl'), record('third')); + + const pushed = await session('push', '--source', 'claude-code', '--all', '-y', '--repo-root', b); + + expect(pushed.stderr).toContain('Not archived: The claude-code session 22222222-2222-4222-8222-222222222222'); + expect(pushed.stderr).toContain('1 session(s) could not be read and were not archived.'); + expect(pushed.stdout).toContain('Pushed 2 session(s) from claude-code'); + expect(pushed.code).toBe(1); + const fresh = clone('fresh'); + expect(indexedSessionIds(fresh)).toEqual(['a1', 'b1', 's0']); + expect(git(fresh, 'log', '-1', '--format=%s', 'HEAD^1')).toBe('sync: push 2 session(s) from claude-code (all workspaces)'); + }); +}); + describe('SyncManager.gitPush', () => { it('pulls once when the remote moved on and leaves a merge stopped on the index to the caller', () => { useGitConfig(); From 657a795e72fc79b285f0939b89c784f815c154f1 Mon Sep 17 00:00:00 2001 From: Saul Moro Date: Thu, 15 Oct 2026 01:06:16 +0200 Subject: [PATCH 5/6] fix(session): pull after finishing an earlier index merge, and take the identity from a side that recorded one (#1133) --- docs/dev/agent-sessions.md | 2 +- src/__tests__/session-sync-pull.test.ts | 49 ++++++++++++++++++++++++- src/session-flow/session-cmd.ts | 24 +++++++----- src/session-flow/sync.ts | 10 +++-- 4 files changed, 70 insertions(+), 15 deletions(-) diff --git a/docs/dev/agent-sessions.md b/docs/dev/agent-sessions.md index 7fd981162..f8db7b9cd 100644 --- a/docs/dev/agent-sessions.md +++ b/docs/dev/agent-sessions.md @@ -14,7 +14,7 @@ Each rule below spans more than one file. The rule names the code that owns it, - **List through `AgentSession.list`.** A caller that wants a member's sessions (`session migrate`, `session push`, a scorer, the dashboard) calls `AgentSession.list({ platforms?, cwd?, updatedSince? })` in `agent-session.ts`, never an adapter's `listConversations`. Each listed session is a ref that `find` reads as is. A session whose first lines do not parse is still listed, under a placeholder title, so `find` reports it `unreadable`, not `not_found`. A tool that cannot be listed is one failure value (`no_adapter`, `tool_unavailable`, or `list_failed`) next to the other tools' sessions. OpenCode's CLI lists one project at a time, and inside a git repo that project is the whole repo, so its adapter keeps only the sessions of the workspace asked for. Without `cwd`, OpenCode is `list_failed`, rather than returning the workspace the process runs in as if it were all of them. Its listing has no message count or size, so those fields are absent rather than 0. - **Stores follow the recorded tool root.** Claude Code and Codex keep their sessions under the root the member recorded with `teamai init` (`toolRoots`), resolved by `resolveMemberToolRoots` and `resolveToolRootDir`, never from `CLAUDE_CONFIG_DIR` or `CODEX_HOME` at run time. `AgentSession` and the `session` commands that build an adapter themselves (`resume`, `rollback`, `platforms`) resolve the roots with `recordedToolRoots` in `agent-session.ts` for the workspace at hand and pass them through the adapter registry (`adapters/index.ts`) to `getClaudeCodeProjectsDir` and `getCodexSessionsDir` in `fs.ts`. A run that reads or copies many sessions passes one `toolRootsLookup()`, so each workspace is resolved once. With nothing recorded they stay `~/.claude/projects` and `~/.codex/sessions`. The variants (`claude-internal`, `codex-internal`, ...) keep their fixed paths. - **The driver picks how a session continues.** The driver that runs a workflow step decides: the same tool on the same machine resumes the session by id; a driver with its own way to move a session uses it; otherwise the local driver copies it with `AgentSession.copy`; failing those, the step starts a fresh session. The workflow engine never copies a session. A step's report carries the tool and that tool's native session id. -- **Session index lock.** Every write to a team repo archive (`saveSession`: dedup lookup, name choice, file writes, `_index.json` upsert) and every index rebuild (`rebuildIndex`, `rebuildAllIndexes`) runs inside `withSessionIndexLock(repoRoot, fn)`. The async command callers take it; `SyncManager` stays synchronous and never takes it, so it cannot re-enter. The lock file is `~/.teamai/locks/sessions-.lock`, keyed by the real path of the team repo's `sessions/`. It stays out of the team repo because there the archive commit could stage it and `pull --all`'s directory renames could move it while held. It covers one machine only. Two machines that both wrote `_index.json` diverge there: `SyncManager.gitPull` (also the retry inside `gitPush`) merges and returns the conflicted indexes, and the command caller rebuilds them and commits the merge (`commitRebuiltIndexes`) under the lock, with the network pull and push outside it. When the lock stays busy the merge is left in progress, and the next `teamai session pull` finishes it. +- **Session index lock.** Every write to a team repo archive (`saveSession`: dedup lookup, name choice, file writes, `_index.json` upsert) and every index rebuild (`rebuildIndex`, `rebuildAllIndexes`) runs inside `withSessionIndexLock(repoRoot, fn)`. The async command callers take it; `SyncManager` stays synchronous and never takes it, so it cannot re-enter. The lock file is `~/.teamai/locks/sessions-.lock`, keyed by the real path of the team repo's `sessions/`. It stays out of the team repo because there the archive commit could stage it and `pull --all`'s directory renames could move it while held. It covers one machine only. Two machines that both wrote `_index.json` diverge there: `SyncManager.gitPull` (also the retry inside `gitPush`) merges and returns the conflicted indexes, and the command caller rebuilds them and commits the merge (`commitRebuiltIndexes`) under the lock, with the network pull and push outside it. When the lock stays busy the merge is left in progress. The next `teamai session pull` finishes it first, then runs its own pull. ## Tests diff --git a/src/__tests__/session-sync-pull.test.ts b/src/__tests__/session-sync-pull.test.ts index e6d4f41c1..3ef14b3bd 100644 --- a/src/__tests__/session-sync-pull.test.ts +++ b/src/__tests__/session-sync-pull.test.ts @@ -216,7 +216,7 @@ describe('SyncManager.gitPull', () => { const result = pullAndRebuild(b); - expect(result).toEqual({ status: 'indexes-conflicted', indexes: [INDEX] }); + expect(result).toEqual({ status: 'indexes-conflicted', indexes: [INDEX], pending: true }); expect(indexedSessionIds(b)).toEqual(['a1', 'b1', 's0']); expect(git(b, 'status', '--porcelain')).toBe(''); }); @@ -308,6 +308,37 @@ describe('SyncManager.gitPull', () => { expect(git(b, 'diff', '--name-only')).toBe(INDEX); }); + // Each side's index as committed: 'kept' leaves its repoIdentity, anything else replaces it (undefined drops it). + it.each([ + ['ours has none, theirs has it', undefined, 'kept'], + ['ours is null, theirs has it', null, 'kept'], + ['ours is empty, theirs has it', '', 'kept'], + ['ours is not a string, theirs has it', 42, 'kept'], + ['ours has it, theirs has none', 'kept', undefined], + ['neither has one, so the directory name decides', undefined, null], + ])('takes the repo identity from a side that recorded one (%s)', (_label, ours, theirs) => { + useGitConfig(); + const { a, b } = seedRemote(); + const commitIndex = (clone: string, sessionId: string, author: string, identity: unknown): void => { + archive(clone, sessionId, author); + if (identity === 'kept') return; + const file = path.join(clone, INDEX); + const index = JSON.parse(fs.readFileSync(file, 'utf-8')) as Record; + if (identity === undefined) delete index.repoIdentity; + else index.repoIdentity = identity; + fs.writeFileSync(file, JSON.stringify(index, null, 2)); + git(clone, 'commit', '-qam', `edit ${sessionId} index`); + }; + commitIndex(a, 'a1', 'alice', theirs); + git(a, 'push', '-q', 'origin', 'main'); + commitIndex(b, 'b1', 'bob', ours); + + expect(pullAndRebuild(b).status).toBe('indexes-conflicted'); + + const index = JSON.parse(fs.readFileSync(path.join(b, INDEX), 'utf-8')) as { repoIdentity: unknown }; + expect(index.repoIdentity).toBe(REPO); + }); + it('keeps the identity a legacy directory name cannot encode', () => { useGitConfig(); const identity = 'github.com/acme/my_app'; @@ -359,6 +390,22 @@ describe('SyncManager.gitPull', () => { }); describe('session pull', () => { + it('finishes a merge an earlier pull left on the session index, then pulls what the remote got since', async () => { + useGitConfig(); + const { a, b } = divergeOnIndex(); + expect(() => git(b, '-c', 'pull.rebase=false', 'pull', '-q', 'origin', 'main')).toThrow(); + archive(a, 'a2', 'alice'); + git(a, 'push', '-q', 'origin', 'main'); + + const pulled = await session('pull', '--all', '--repo-root', b); + + expect(pulled.code).toBe(0); + expect(() => git(b, 'merge-base', '--is-ancestor', 'origin/main', 'HEAD')).not.toThrow(); + expect(git(b, 'rev-parse', 'origin/main')).toBe(git(a, 'rev-parse', 'HEAD')); + expect(sessionIdsOf(git(b, 'show', `HEAD:${INDEX}`))).toEqual(['a1', 'a2', 'b1', 's0']); + expect(() => git(b, 'rev-parse', '-q', '--verify', 'MERGE_HEAD')).toThrow(); + }); + it('leaves a merge stopped on the session index for a rerun while another command holds the archive lock', async () => { useGitConfig(); const { b } = divergeOnIndex(); diff --git a/src/session-flow/session-cmd.ts b/src/session-flow/session-cmd.ts index f7bc072c0..4a04dbcf1 100644 --- a/src/session-flow/session-cmd.ts +++ b/src/session-flow/session-cmd.ts @@ -1111,22 +1111,28 @@ export function registerSessionFlowCommands(sessionCmd: Command): void { return; } // [fixed] a gitPull failure (no remote / repoRoot does not exist) used to crash with a raw stack trace - const pulled = runGitStep(() => syncMgr.gitPull(), repoRoot, 'git pull'); - if (pulled.status === 'stopped') { - console.error(`Error: ${pulled.reason}`); - process.exit(1); - } - if (pulled.status === 'indexes-conflicted') { - // The rebuild takes the lock and the network pull above does not: a lock held across it would make other saves wait and fail busy. + const finishIndexMerge = async (indexes: string[]): Promise => { + // The rebuild takes the lock and the network pull does not: a lock held across it would make other saves wait and fail busy. const merged = await withSessionIndexLock(repoRoot, async () => - runGitStep(() => syncMgr.commitRebuiltIndexes(pulled.indexes), repoRoot, 'git pull'), + runGitStep(() => syncMgr.commitRebuiltIndexes(indexes), repoRoot, 'git pull'), ); if (merged.status === 'busy') { console.error(`Error: ${mergeBusyMessage(repoRoot, merged.lockPath)}`); process.exit(1); } - console.log(` ✓ Merged remote sessions and rebuilt ${pulled.indexes.length} conflicted session index(es)`); + console.log(` ✓ Merged remote sessions and rebuilt ${indexes.length} conflicted session index(es)`); + }; + let pulled = runGitStep(() => syncMgr.gitPull(), repoRoot, 'git pull'); + // A merge an earlier pull left on the session indexes is finished first; it does not replace this pull. + if (pulled.status === 'indexes-conflicted' && pulled.pending) { + await finishIndexMerge(pulled.indexes); + pulled = runGitStep(() => syncMgr.gitPull(), repoRoot, 'git pull'); + } + if (pulled.status === 'stopped') { + console.error(`Error: ${pulled.reason}`); + process.exit(1); } + if (pulled.status === 'indexes-conflicted') await finishIndexMerge(pulled.indexes); if (opts.all) { // P2: rebuild by directory (including repos whose index is missing or corrupt — those are exactly the ones that most need repair), // rather than only rebuilding the ones whose identity can be recovered from _index.json. diff --git a/src/session-flow/sync.ts b/src/session-flow/sync.ts index 1428e27f4..804b2cece 100644 --- a/src/session-flow/sync.ts +++ b/src/session-flow/sync.ts @@ -319,11 +319,12 @@ export type GitCommitResult = /** * The outcomes of `gitPull`. `stopped` merged nothing, so the checkout is as it was before the pull. * `indexes-conflicted` leaves the merge in progress with only these session indexes unmerged: the caller - * finishes it with `commitRebuiltIndexes` under `withSessionIndexLock`. + * finishes it with `commitRebuiltIndexes` under `withSessionIndexLock`. `pending` means an earlier pull left + * that merge: the requested pull has not run yet, so the caller pulls again once the merge is finished. */ export type PullResult = | { status: 'pulled' } - | { status: 'indexes-conflicted'; indexes: string[] } + | { status: 'indexes-conflicted'; indexes: string[]; pending?: true } | { status: 'stopped'; reason: string }; /** The outcomes of `gitPush`. `indexes-conflicted` is the retry's pull, left for the caller as in `PullResult`. */ @@ -1170,7 +1171,7 @@ export class SyncManager { : 'Resolve and `git add` them and run `git commit`, or run `git merge --abort`, then retry'; return { status: 'stopped', reason: `A merge is in progress in ${this.repoRoot} with conflicts in ${others.join(', ')}. ${next}.` }; } - return { status: 'indexes-conflicted', indexes: unmerged }; + return { status: 'indexes-conflicted', indexes: unmerged, pending: true }; } if (unmerged.length > 0) { return { @@ -1218,7 +1219,8 @@ export class SyncManager { private conflictedIndexIdentity(index: string): string | null { for (const stage of [2, 3]) { try { - return (JSON.parse(this.runGit(['show', `:${stage}:./${index}`])) as RepoIndex).repoIdentity; + const { repoIdentity } = JSON.parse(this.runGit(['show', `:${stage}:./${index}`])) as { repoIdentity?: unknown }; + if (typeof repoIdentity === 'string' && repoIdentity) return repoIdentity; } catch { // that side deleted the index or left it unparsable } From 8d8c66b5e8c67fcd055bc8df5dc2550b023cae2f Mon Sep 17 00:00:00 2001 From: Saul Moro Date: Thu, 15 Oct 2026 06:07:46 +0200 Subject: [PATCH 6/6] test(session): a pull that merges a diverged index leaves the checkout clean (#1133) --- src/__tests__/session-sync-pull.test.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/__tests__/session-sync-pull.test.ts b/src/__tests__/session-sync-pull.test.ts index 3ef14b3bd..191fdac3a 100644 --- a/src/__tests__/session-sync-pull.test.ts +++ b/src/__tests__/session-sync-pull.test.ts @@ -404,6 +404,10 @@ describe('session pull', () => { expect(git(b, 'rev-parse', 'origin/main')).toBe(git(a, 'rev-parse', 'HEAD')); expect(sessionIdsOf(git(b, 'show', `HEAD:${INDEX}`))).toEqual(['a1', 'a2', 'b1', 's0']); expect(() => git(b, 'rev-parse', '-q', '--verify', 'MERGE_HEAD')).toThrow(); + expect(git(b, 'status', '--porcelain')).toBe(''); + const merged = fs.readFileSync(path.join(b, INDEX), 'utf-8'); + expect((await session('pull', '--all', '--repo-root', b)).code).toBe(0); + expect(fs.readFileSync(path.join(b, INDEX), 'utf-8')).toBe(merged); }); it('leaves a merge stopped on the session index for a rerun while another command holds the archive lock', async () => { @@ -425,6 +429,7 @@ describe('session pull', () => { expect(rerun.stdout).toContain('Merged remote sessions and rebuilt 1 conflicted session index(es)'); expect(() => git(b, 'rev-parse', '-q', '--verify', 'MERGE_HEAD')).toThrow(); expect(sessionIdsOf(git(b, 'show', `HEAD:${INDEX}`))).toEqual(['a1', 'b1', 's0']); + expect(git(b, 'status', '--porcelain')).toBe(''); }); });