diff --git a/engine/bun.lock b/engine/bun.lock index e5595f37..ed8dcb31 100644 --- a/engine/bun.lock +++ b/engine/bun.lock @@ -14,6 +14,7 @@ }, "devDependencies": { "@biomejs/biome": "^2.0.0", + "@types/bun": "^1.3.14", "@types/node": "^22.0.0", "typescript": "^5.6.0", }, @@ -196,6 +197,8 @@ "@tokenizer/token": ["@tokenizer/token@0.3.0", "", {}, "sha512-OvjF+z51L3ov0OyAU0duzsYuvO01PH7x4t6DJx+guahgTnBHkhJdG7soQeTSFLWN3efnHyibZ4Z8l2EuWwJN3A=="], + "@types/bun": ["@types/bun@1.3.14", "", { "dependencies": { "bun-types": "1.3.14" } }, "sha512-h1hFqFVcvAvD9j9K7ZW7vd82aSA+rTdznZa+5bwvCwqSB1jmmfLcbIWhOLx1/+boy/xmjgCs/OMUL8hRJSmnPw=="], + "@types/node": ["@types/node@22.19.19", "", { "dependencies": { "undici-types": "~6.21.0" } }, "sha512-dyh/xO2Fh5bYrfWaaqGrRQQGkNdmYw6AmaAUvYeUMNTWQtvb796ikLdmTchRmOlOiIJ1TDXfWgVx1QkUlQ6Hew=="], "@types/retry": ["@types/retry@0.12.0", "", {}, "sha512-wWKOClTTiizcZhXnPY4wikVAwmdYHp8q6DmC+EJUzAMsycb7HB32Kh9RN4+0gExjmPmZSAQjgURXIGATPegAvA=="], @@ -226,6 +229,8 @@ "buffer-equal-constant-time": ["buffer-equal-constant-time@1.0.1", "", {}, "sha512-zRpUiDwd/xk6ADqPMATG8vc9VPrkck7T07OIx0gnjmJAnHnTVXNQG3vfvWNuiZIkwu9KrKdA1iJKfsfTVxE6NA=="], + "bun-types": ["bun-types@1.3.14", "", { "dependencies": { "@types/node": "*" } }, "sha512-4N0ig0fEomHt5R0KCFWjovxow98rIoRwKolrYdCcknNwMekCXRnWEUvgu5soYV8QXtVsrUD8B95MBOZGPvr6KQ=="], + "bytes": ["bytes@3.1.2", "", {}, "sha512-/Nf7TyzTx6S3yRJObOAV7956r8cr2+Oj8AC5dt8wSP3BQAoeX58NoHyCU8P8zGkNXStjTSi6fzO6F0pBdcYbEg=="], "call-bind-apply-helpers": ["call-bind-apply-helpers@1.0.2", "", { "dependencies": { "es-errors": "^1.3.0", "function-bind": "^1.1.2" } }, "sha512-Sp1ablJ0ivDkSzjcaJdxEunN5/XvksFJ2sMBFfq6x0ryhQV/2b/KwFe21cMpmHtPOSij8K99/wSfoEuTObmuMQ=="], diff --git a/engine/package.json b/engine/package.json index 076ccd44..f8e933e0 100644 --- a/engine/package.json +++ b/engine/package.json @@ -28,6 +28,7 @@ }, "devDependencies": { "@biomejs/biome": "^2.0.0", + "@types/bun": "^1.3.14", "@types/node": "^22.0.0", "typescript": "^5.6.0" } diff --git a/engine/src/agent.fork.test.ts b/engine/src/agent.fork.test.ts index cc08e9be..c2bd3d29 100644 --- a/engine/src/agent.fork.test.ts +++ b/engine/src/agent.fork.test.ts @@ -70,10 +70,14 @@ test('the source session file is never written — even when the fork is appende fs.mkdtempSync(path.join(tmp, 'target-')), sessionDir, ); + // Cast at the SDK boundary, not around our own types. pi's exported `Message` union is + // narrower than what `appendMessage` accepts at runtime — this shape is what the agent + // actually sends and what the assertions below verify the SDK does with it. The cast + // documents that mismatch rather than hiding a shape our code got wrong. forked.appendMessage({ role: 'user', content: [{ type: 'text', text: 'a question against the frozen snapshot' }], - }); + } as Parameters[0]); expect(fs.readFileSync(source, 'utf8')).toBe(before); // byte-identical expect(fs.readFileSync(forked.getSessionFile() as string, 'utf8')).toContain( diff --git a/engine/src/kild/attachment.test.ts b/engine/src/kild/attachment.test.ts index c5a9f0f0..04efce6d 100644 --- a/engine/src/kild/attachment.test.ts +++ b/engine/src/kild/attachment.test.ts @@ -161,7 +161,7 @@ describe('concurrent attaches', () => { path.join(home, 'attached', 'claims', 'kild-w', 'kild'), 'utf8', ); - expect(claim.trim()).toBe(winner); + expect(claim.trim()).toBe(winner as string); expect(await findAttachment(winner as string)).toMatchObject({ kildId: 'kild-w' }); }); diff --git a/engine/src/kild/git-review.test.ts b/engine/src/kild/git-review.test.ts index 61d98130..16143e57 100644 --- a/engine/src/kild/git-review.test.ts +++ b/engine/src/kild/git-review.test.ts @@ -72,7 +72,7 @@ test('commits vs base come newest-first with per-commit stats', async () => { await git(dir, ['add', '.']); await commit(dir, 'trim a, add b'); - const result = await reviewCommits(dir, 'main'); + const result = await reviewCommits(dir, { base: 'main', source: 'explicit' }); expect(result.error).toBeUndefined(); expect(result.base).toBe('main'); @@ -93,21 +93,21 @@ test('commits vs base come newest-first with per-commit stats', async () => { test('no commits ahead of base yields an empty list, no error', async () => { const dir = await initRepo(); - const result = await reviewCommits(dir, 'main'); + const result = await reviewCommits(dir, { base: 'main', source: 'explicit' }); expect(result.error).toBeUndefined(); expect(result.commits).toEqual([]); }); test('commits: a missing base ref is an error object, not a crash', async () => { const dir = await initRepo(); - const result = await reviewCommits(dir, 'does-not-exist'); + const result = await reviewCommits(dir, { base: 'does-not-exist', source: 'explicit' }); expect(result.error).toBe('base ref not found: does-not-exist'); expect(result.commits).toEqual([]); }); test('commits: a non-git directory is an error object, not a crash', async () => { const dir = mkTmp('kild-git-review-nogit-'); - const result = await reviewCommits(dir, 'main'); + const result = await reviewCommits(dir, { base: 'main', source: 'explicit' }); expect(result.error).toBeDefined(); expect(result.commits).toEqual([]); }); @@ -123,7 +123,7 @@ test('files combine committed, uncommitted, and untracked changes vs base', asyn fs.writeFileSync(path.join(dir, 'README.md'), 'hello\nedited\n'); // uncommitted edit fs.writeFileSync(path.join(dir, 'untracked.txt'), 'u1\nu2\nu3\n'); // never added - const result = await reviewFiles(dir, 'main'); + const result = await reviewFiles(dir, { base: 'main', source: 'explicit' }); expect(result.error).toBeUndefined(); const byPath = new Map(result.files.map((file) => [file.path, file])); @@ -160,7 +160,7 @@ test('files report deletions and renames with the pre-rename path', async () => await git(dir, ['mv', 'README.md', 'RENAMED.md']); await commit(dir, 'delete + rename'); - const result = await reviewFiles(dir, 'main'); + const result = await reviewFiles(dir, { base: 'main', source: 'explicit' }); expect(result.error).toBeUndefined(); const byPath = new Map(result.files.map((file) => [file.path, file])); @@ -184,7 +184,7 @@ test("files never include the base's own advances (merge-base semantics)", async await commit(dir, 'theirs'); await git(dir, ['checkout', 'feature']); - const result = await reviewFiles(dir, 'main'); + const result = await reviewFiles(dir, { base: 'main', source: 'explicit' }); expect(result.error).toBeUndefined(); expect(result.files.map((file) => file.path)).toEqual(['mine.txt']); @@ -192,7 +192,7 @@ test("files never include the base's own advances (merge-base semantics)", async test('files: a missing base ref is an error object, not a crash', async () => { const dir = await initRepo(); - const result = await reviewFiles(dir, 'does-not-exist'); + const result = await reviewFiles(dir, { base: 'does-not-exist', source: 'explicit' }); expect(result.error).toBe('base ref not found: does-not-exist'); expect(result.files).toEqual([]); }); @@ -207,7 +207,7 @@ test('diff returns one unified patch covering committed + working-tree changes', await commit(dir, 'committed line'); fs.writeFileSync(path.join(dir, 'README.md'), 'hello\ncommitted\nuncommitted\n'); - const result = await reviewDiff(dir, 'main', 'README.md'); + const result = await reviewDiff(dir, { base: 'main', source: 'explicit' }, 'README.md'); expect(result.error).toBeUndefined(); expect(result.truncated).toBe(false); @@ -220,7 +220,7 @@ test('diff covers an untracked file via no-index', async () => { const dir = await initRepo(); fs.writeFileSync(path.join(dir, 'fresh.txt'), 'brand new\n'); - const result = await reviewDiff(dir, 'main', 'fresh.txt'); + const result = await reviewDiff(dir, { base: 'main', source: 'explicit' }, 'fresh.txt'); expect(result.error).toBeUndefined(); expect(result.patch).toContain('+brand new'); @@ -229,7 +229,7 @@ test('diff covers an untracked file via no-index', async () => { test('diff refuses a path git did not report (traversal guard)', async () => { const dir = await initRepo(); for (const evil of ['../../etc/passwd', '/etc/passwd', 'nope.txt']) { - const result = await reviewDiff(dir, 'main', evil); + const result = await reviewDiff(dir, { base: 'main', source: 'explicit' }, evil); expect(result.unknownPath).toBe(true); expect(result.error).toContain('not reported by git'); expect(result.patch).toBe(''); @@ -240,7 +240,7 @@ test('diff larger than the cap is truncated and flagged', async () => { const dir = await initRepo(); fs.writeFileSync(path.join(dir, 'big.txt'), 'x-line-of-payload\n'.repeat(20_000)); // ~360 KB - const result = await reviewDiff(dir, 'main', 'big.txt'); + const result = await reviewDiff(dir, { base: 'main', source: 'explicit' }, 'big.txt'); expect(result.error).toBeUndefined(); expect(result.truncated).toBe(true); diff --git a/engine/src/kild/git-review.ts b/engine/src/kild/git-review.ts index e6a86afe..d721b256 100644 --- a/engine/src/kild/git-review.ts +++ b/engine/src/kild/git-review.ts @@ -3,7 +3,7 @@ import fs from 'node:fs/promises'; import path from 'node:path'; import { promisify } from 'node:util'; -import { resolveDefaultBase } from './worktree-status.ts'; +import { type BaseSource, type ResolvedBase, resolveDefaultBase } from './worktree-status.ts'; /** * Review intelligence — the git drill-down behind a review surface. Where @@ -56,12 +56,18 @@ export interface ReviewFile { export interface ReviewCommitsResult { base: string; + /** Where {@link base} came from — see BaseSource. A commit count measured against a guessed + * base is not evidence of unlanded work, and a caller refusing on it must be able to say so. */ + baseSource: BaseSource; commits: ReviewCommit[]; error?: string; // any git failure captured here, NEVER thrown } export interface ReviewFilesResult { base: string; + /** Where {@link base} came from — same contract as ReviewCommitsResult. Stats measured + * against a guessed base are not facts about anybody's work either. */ + baseSource: BaseSource; files: ReviewFile[]; error?: string; // any git failure captured here, NEVER thrown } @@ -221,9 +227,17 @@ export function parsePorcelainZ(stdout: string): { /** Commits on the kild's branch that base doesn't have (`base..HEAD`), newest * first, each with its own diff stats. Never throws — failures land in `error`. */ -export async function reviewCommits(dir: string, base?: string): Promise { - const resolvedBase = base ?? (await resolveDefaultBase(dir)); - const result: ReviewCommitsResult = { base: resolvedBase, commits: [] }; +export async function reviewCommits( + dir: string, + base?: ResolvedBase, +): Promise { + const resolved = base ?? (await resolveDefaultBase(dir)); + const resolvedBase = resolved.base; + const result: ReviewCommitsResult = { + base: resolvedBase, + baseSource: resolved.source, + commits: [], + }; const invalid = await verifyRepoAndBase(dir, resolvedBase); if (invalid) { result.error = invalid; @@ -269,9 +283,10 @@ async function countLines(dir: string, file: string): Promise { /** Per-file diff stats vs base — committed (branch vs merge-base) and uncommitted * (working tree, incl. untracked files) combined into one list. Never throws. */ -export async function reviewFiles(dir: string, base?: string): Promise { - const resolvedBase = base ?? (await resolveDefaultBase(dir)); - const result: ReviewFilesResult = { base: resolvedBase, files: [] }; +export async function reviewFiles(dir: string, base?: ResolvedBase): Promise { + const resolved = base ?? (await resolveDefaultBase(dir)); + const resolvedBase = resolved.base; + const result: ReviewFilesResult = { base: resolvedBase, baseSource: resolved.source, files: [] }; const invalid = await verifyRepoAndBase(dir, resolvedBase); if (invalid) { result.error = invalid; @@ -358,7 +373,7 @@ async function noIndexDiff(dir: string, file: string): Promise { * and no git/fs call ever receives the raw caller path otherwise. Never throws. */ export async function reviewDiff( dir: string, - base: string | undefined, + base: ResolvedBase | undefined, file: string, ): Promise { const files = await reviewFiles(dir, base); diff --git a/engine/src/kild/kild-close.ts b/engine/src/kild/kild-close.ts index c9576994..ab1173d5 100644 --- a/engine/src/kild/kild-close.ts +++ b/engine/src/kild/kild-close.ts @@ -3,6 +3,7 @@ import path from 'node:path'; import { configuredCloseHook, configuredMemoryDir } from './config.ts'; import { type HookAgentSpawn, type KildCloseEvent, runCloseHook } from './hooks.ts'; import type { Kild } from './kild-types.ts'; +import { storedBase } from './kild-types.ts'; import { appendKildLog, collectLedgerFacts, kildTranscriptPath } from './memory.ts'; import { worktreePath } from './worktree.ts'; @@ -47,7 +48,7 @@ export async function closeKild(kild: Kild, deps: CloseDeps): Promise { appendKildLog( kild, memoryDir, - await collectLedgerFacts(dir, kild.base, kild.landedSha, kild.landed), + await collectLedgerFacts(dir, storedBase(kild), kild.landedSha, kild.landed), ); } catch (err) { console.error(`kild: ledger append failed for '${kild.name}': ${errText(err)}`); diff --git a/engine/src/kild/kild-disposal.test.ts b/engine/src/kild/kild-disposal.test.ts index 9cc1e286..aea28e5d 100644 --- a/engine/src/kild/kild-disposal.test.ts +++ b/engine/src/kild/kild-disposal.test.ts @@ -53,7 +53,7 @@ test('a tree with ONLY untracked litter is disposable, and the litter is named', repo, dir: wt.path, branch: 'kild/litter', - base: 'main', + base: { base: 'main', source: 'explicit' }, inUse: false, }); expect(assessment.ok).toBe(true); @@ -77,7 +77,7 @@ test('uncommitted changes to TRACKED files are not a refusal either', async () = repo, dir: wt.path, branch: 'kild/tracked', - base: 'main', + base: { base: 'main', source: 'explicit' }, inUse: false, }); expect(assessment.ok).toBe(true); @@ -98,7 +98,7 @@ test('an undeterminable discard list says so, instead of reporting nothing lost' repo, dir: wt.path, branch: 'kild/unreadable', - base: 'main', + base: { base: 'main', source: 'explicit' }, inUse: false, force: true, // past the commits guard, which refuses `undetermined` on its own }); @@ -119,7 +119,7 @@ test('a branch carrying commits base does not have is REFUSED, with the count an repo, dir: wt.path, branch: 'kild/authored', - base: 'main', + base: { base: 'main', source: 'explicit' }, inUse: false, }); expect(assessment.ok).toBe(false); @@ -143,7 +143,7 @@ test('force overrides the authored refusal — the branch and its commits surviv repo, dir: wt.path, branch: 'kild/forced', - base: 'main', + base: { base: 'main', source: 'explicit' }, inUse: false, force: true, }); @@ -183,7 +183,7 @@ test('a base that cannot be resolved is undetermined, and says force is the way repo, dir: wt.path, branch: 'kild/nobase', - base: 'no-such-base', + base: { base: 'no-such-base', source: 'explicit' }, inUse: false, }); expect(assessment).toMatchObject({ ok: false, code: 'undetermined' }); @@ -195,7 +195,7 @@ test('a base that cannot be resolved is undetermined, and says force is the way repo, dir: wt.path, branch: 'kild/nobase', - base: 'no-such-base', + base: { base: 'no-such-base', source: 'explicit' }, inUse: false, force: true, }), @@ -209,3 +209,68 @@ test('removal frees the tree and keeps the branch — disposal never deletes wor expect(existsSync(wt.path)).toBe(false); expect((await git('branch')).stdout).toContain('kild/reclaim'); }); + +test('a refusal built on a GUESSED base says so, and one on a chosen base does not', async () => { + // The measured failure: 116 trees held behind "carries 365 commits not in main". True, and + // useless — that repo's default is `dev`, so every tree was hundreds of commits ahead of a + // branch it had never forked from, by construction. The refusal named the base. What it + // could not say was that nobody had chosen it, and that cost four measurement passes and + // 5.4 GB. The count is only evidence of unlanded work if the base is. + const wt = await ensureWorktree(repo, 'guessed', 'main'); + writeFileSync(path.join(wt.path, 'a.txt'), 'authored\n'); + await gitIn(wt.path, 'add', '.'); + await gitIn(wt.path, 'commit', '-q', '-m', 'real work'); + + // No `base` given, and this repo has no origin/HEAD → the literal fallback. + const guessed = await assessDisposal({ + repo, + dir: wt.path, + branch: 'kild/guessed', + inUse: false, + }); + expect(guessed.ok).toBe(false); + if (guessed.ok) return; + expect(guessed.baseSource).toBe('fallback'); + expect(guessed.base).toBe('main'); + expect(guessed.message).toContain('nothing configured a base'); + expect(guessed.message).toContain('.kild/config.json'); + + // Named explicitly: the same refusal, with no hedge, because somebody chose the branch. + const chosen = await assessDisposal({ + repo, + dir: wt.path, + branch: 'kild/guessed', + base: { base: 'main', source: 'explicit' }, + inUse: false, + }); + expect(chosen.ok).toBe(false); + if (chosen.ok) return; + expect(chosen.baseSource).toBe('explicit'); + expect(chosen.message).not.toContain('this is a guess'); + expect(chosen.message).toContain('carries 1 commit not in main'); +}); + +test('a base cached in origin/HEAD is labelled as the cache it is', async () => { + // origin/HEAD is written at clone time and never refreshed, so it goes stale silently when + // the remote's default moves. Better than a literal guess; still not a fact anyone asserted. + await git('remote', 'add', 'origin', repo); + await git('symbolic-ref', 'refs/remotes/origin/HEAD', 'refs/remotes/origin/main'); + await git('update-ref', 'refs/remotes/origin/main', 'main'); + + const wt = await ensureWorktree(repo, 'cached', 'main'); + writeFileSync(path.join(wt.path, 'a.txt'), 'authored\n'); + await gitIn(wt.path, 'add', '.'); + await gitIn(wt.path, 'commit', '-q', '-m', 'real work'); + + const assessment = await assessDisposal({ + repo, + dir: wt.path, + branch: 'kild/cached', + inUse: false, + }); + expect(assessment.ok).toBe(false); + if (assessment.ok) return; + expect(assessment.baseSource).toBe('origin-head'); + expect(assessment.message).toContain('origin/HEAD'); + expect(assessment.message).toContain('caches at clone time'); +}); diff --git a/engine/src/kild/kild-disposal.ts b/engine/src/kild/kild-disposal.ts index ca070c80..27d7e74f 100644 --- a/engine/src/kild/kild-disposal.ts +++ b/engine/src/kild/kild-disposal.ts @@ -1,5 +1,6 @@ import { reviewCommits } from './git-review.ts'; import { changedFiles, forceRemoveWorktree, registeredWorktree } from './worktree.ts'; +import type { BaseSource, ResolvedBase } from './worktree-status.ts'; /** * Disposal — the verb that reclaims a kild's worktree, and the one guard it answers to. @@ -31,6 +32,10 @@ export interface DisposalRefusal { commits?: number; /** For `authored`: short sha of the newest of them. */ tip?: string; + /** For `authored`: the branch the count was measured against, and whether anybody chose it. + * On the wire so a client can render "measured against a guess" rather than a bare number. */ + base?: string; + baseSource?: BaseSource | 'explicit'; } /** A disposal the guard allows, and the cost of going ahead with it. */ @@ -65,7 +70,7 @@ export interface DisposalRequest { /** Branch the tree is on (`kild/`), for the message. */ branch?: string; /** Base branch authored commits are measured against. Absent → the repo's default. */ - base?: string; + base?: ResolvedBase; /** True when a live agent process is working in this tree. */ inUse: boolean; /** Dispose even when the branch carries authored commits. The branch — and every commit @@ -107,15 +112,30 @@ export async function assessDisposal(req: DisposalRequest): Promise 0 && !req.force) { + // Say where the base came from when nobody chose it. "carries 365 commits not in main" is + // a true sentence and a useless one if `main` was a guess — that exact refusal held 116 + // trees and 5.4 GB, because a repo whose default was `dev` read as hundreds of commits + // ahead of a branch it never forked from. The count is not evidence of unlanded work + // unless the base is, and only the engine knows which it had. + const guessed = + review.baseSource === 'origin-head' + ? ` (base ${review.base} came from origin/HEAD, which git caches at clone time and ` + + 'never refreshes — set baseBranch in .kild/config.json if it is wrong)' + : review.baseSource === 'fallback' + ? ` (nothing configured a base, so this is a guess — set baseBranch in ` + + '.kild/config.json, or pass --base)' + : ''; return { ok: false, code: 'authored', message: `${where} carries ${commits} commit${commits === 1 ? '' : 's'} not in ` + - `${review.base}${tip ? ` (tip ${tip})` : ''} — land it, or retry with force ` + + `${review.base}${tip ? ` (tip ${tip})` : ''}${guessed} — land it, or retry with force ` + '(the branch and its commits survive either way)', commits, tip, + base: review.base, + baseSource: review.baseSource, }; } diff --git a/engine/src/kild/kild-land.test.ts b/engine/src/kild/kild-land.test.ts index 40d8d45c..772f2725 100644 --- a/engine/src/kild/kild-land.test.ts +++ b/engine/src/kild/kild-land.test.ts @@ -68,7 +68,7 @@ test('the dry run reports what would land and TOUCHES NOTHING', async () => { const beforeTree = await snapshot(wt.path); const beforeRepo = await snapshot(repo); - const plan = await landPlan(wt.path, 'main'); + const plan = await landPlan(wt.path, { base: 'main', source: 'explicit' }); expect(plan.wouldMerge).toBe(true); expect(plan.merged).toBe(false); expect(plan.sha).toBeUndefined(); @@ -89,7 +89,7 @@ test('the dry run names the colliding files and would not merge', async () => { await git('commit', '-q', '-am', 'main moves'); const beforeRepo = await snapshot(repo); - const plan = await landPlan(wt.path, 'main'); + const plan = await landPlan(wt.path, { base: 'main', source: 'explicit' }); expect(plan.wouldMerge).toBe(false); expect(plan.collides).toEqual(['README.md']); expect(plan.error).toContain('conflicts in 1 file'); @@ -99,30 +99,30 @@ test('the dry run names the colliding files and would not merge', async () => { test('a kild with nothing committed would not land, and says exactly that', async () => { const wt = await ensureWorktree(repo, 'empty', 'main'); writeFileSync(path.join(wt.path, 'only-litter.txt'), 'x'); - const plan = await landPlan(wt.path, 'main'); + const plan = await landPlan(wt.path, { base: 'main', source: 'explicit' }); expect(plan.wouldMerge).toBe(false); expect(plan.error).toBe('nothing committed on kild/empty vs main'); }); test('a kild that ran in the checkout has no branch to land', async () => { - const plan = await landPlan(repo, 'main'); + const plan = await landPlan(repo, { base: 'main', source: 'explicit' }); expect(plan.wouldMerge).toBe(false); expect(plan.error).toContain('no branch to land'); }); test('landing merges into base and reports the merge sha', async () => { const wt = await kildWithWork('ship'); - const result = await landMerge(repo, wt.path, 'main'); + const result = await landMerge(repo, wt.path, { base: 'main', source: 'explicit' }); expect(result.merged).toBe(true); expect(result.sha).toMatch(/^[0-9a-f]{40}$/); // The base really carries the work now, at exactly the sha reported. - expect((await git('rev-parse', 'HEAD')).stdout.trim()).toBe(result.sha); + expect((await git('rev-parse', 'HEAD')).stdout.trim()).toBe(result.sha as string); expect((await git('log', '--format=%s', '-1')).stdout.trim()).toBe( 'kild: land kild/ship into main', ); expect((await git('branch', '--merged', 'main')).stdout).toContain('kild/ship'); // And a dry run afterwards agrees there is nothing left to land. - expect((await landPlan(wt.path, 'main')).wouldMerge).toBe(false); + expect((await landPlan(wt.path, { base: 'main', source: 'explicit' })).wouldMerge).toBe(false); }); test('landing refuses when base is not checked out in the repo, and merges nothing', async () => { @@ -130,7 +130,7 @@ test('landing refuses when base is not checked out in the repo, and merges nothi await git('checkout', '-q', '-b', 'side'); const before = await snapshot(repo); - const result = await landMerge(repo, wt.path, 'main'); + const result = await landMerge(repo, wt.path, { base: 'main', source: 'explicit' }); expect(result.merged).toBe(false); expect(result.error).toContain('is on side, not main'); expect(await snapshot(repo)).toEqual(before); @@ -141,7 +141,7 @@ test('landing refuses on a dirty main checkout rather than entangling the merge' writeFileSync(path.join(repo, 'README.md'), 'edited in the checkout\n'); const before = await snapshot(repo); - const result = await landMerge(repo, wt.path, 'main'); + const result = await landMerge(repo, wt.path, { base: 'main', source: 'explicit' }); expect(result.merged).toBe(false); expect(result.error).toContain('uncommitted changes'); expect(await snapshot(repo)).toEqual(before); @@ -153,7 +153,7 @@ test('a conflicting land is refused before git is asked to merge', async () => { await git('commit', '-q', '-am', 'main moves'); const before = await snapshot(repo); - const result = await landMerge(repo, wt.path, 'main'); + const result = await landMerge(repo, wt.path, { base: 'main', source: 'explicit' }); expect(result.merged).toBe(false); expect(result.collides).toEqual(['README.md']); // No half-finished merge left behind for someone else to discover. diff --git a/engine/src/kild/kild-land.ts b/engine/src/kild/kild-land.ts index 22d2bc7f..e1c98f3a 100644 --- a/engine/src/kild/kild-land.ts +++ b/engine/src/kild/kild-land.ts @@ -3,6 +3,7 @@ import { promisify } from 'node:util'; import { type ReviewCommit, reviewCommits } from './git-review.ts'; import { currentBranch } from './worktree.ts'; +import type { ResolvedBase } from './worktree-status.ts'; import { kildGitStatus } from './worktree-status.ts'; /** @@ -91,7 +92,7 @@ export async function conflictingPaths( * `dir` is the kild's effective directory (its worktree, else its cwd); `base` its base * branch (absent → the repo's default). */ -export async function landPlan(dir: string, base?: string): Promise { +export async function landPlan(dir: string, base?: ResolvedBase): Promise { const status = await kildGitStatus(dir, base); const result: LandResult = { base: status.base, @@ -115,7 +116,7 @@ export async function landPlan(dir: string, base?: string): Promise return result; } - const review = await reviewCommits(dir, status.base); + const review = await reviewCommits(dir, { base: status.base, source: status.baseSource }); if (review.error) { result.error = review.error; return result; @@ -147,7 +148,11 @@ export async function landPlan(dir: string, base?: string): Promise * report the merge sha. Runs the same plan first and refuses on anything it flagged, so a * caller never has to interpret two different verdicts. */ -export async function landMerge(repo: string, dir: string, base?: string): Promise { +export async function landMerge( + repo: string, + dir: string, + base?: ResolvedBase, +): Promise { const plan = await landPlan(dir, base); if (!plan.wouldMerge || plan.branch === null) return plan; diff --git a/engine/src/kild/kild-manager.ts b/engine/src/kild/kild-manager.ts index 767c4090..5dce80da 100644 --- a/engine/src/kild/kild-manager.ts +++ b/engine/src/kild/kild-manager.ts @@ -33,9 +33,18 @@ import { type SendOut, type SpawnContext, type StopOut, + storedBase, } from './kild-types.ts'; import { GENERAL_PERSONA, listPersonas } from './personas.ts'; import { resolveBaseBranch, worktreePath } from './worktree.ts'; + +/** The base and its provenance as stored fields — one call, so the two can never be recorded + * apart. A `base` without its `baseSource` is a guess wearing a decision's clothes. */ +async function resolvedBaseFields(cwd: string, flag?: string) { + const { base, source } = await resolveBaseBranch(cwd, flag); + return { base, baseSource: source }; +} + import { kildGitStatus } from './worktree-status.ts'; /** Soft cap on kild size — a cheap loop/scale guard in v1 (loop control is otherwise @@ -179,7 +188,7 @@ export class KildManager { // Resolve the base once here — the single chokepoint every creator (CLI, REST, WS) // flows through: explicit `base` wins, else the cwd's configured `baseBranch`, else // its current branch, else `main`. - base: await resolveBaseBranch(spec.cwd, spec.base), + ...(await resolvedBaseFields(spec.cwd, spec.base)), agents: [], log: [], }; @@ -265,7 +274,10 @@ export class KildManager { landedSha: kild.landedSha, agents: kild.agents.map(agentView), totals: costTotals(kild.agents), - git: await kildGitStatus(kild.worktree ? worktreePath(kild.worktree) : kild.cwd, kild.base), + git: await kildGitStatus( + kild.worktree ? worktreePath(kild.worktree) : kild.cwd, + storedBase(kild), + ), }; } diff --git a/engine/src/kild/kild-registry.test.ts b/engine/src/kild/kild-registry.test.ts index b35303f7..e5f17a36 100644 --- a/engine/src/kild/kild-registry.test.ts +++ b/engine/src/kild/kild-registry.test.ts @@ -32,7 +32,7 @@ function kild(id: string): Kild { } function msg(kildId: string, text: string): Message { - return { id: `${kildId}-1`, kildId, from: 'human', to: ['agent'], text, ts: 1 }; + return { id: `${kildId}-1`, kildId, from: 'human', to: ['agent'], text, ts: 1, seq: 1 }; } test('appendMessage write-throughs the kild log; an empty kild leaves no file', () => { diff --git a/engine/src/kild/kild-types.ts b/engine/src/kild/kild-types.ts index c1145897..8c12913d 100644 --- a/engine/src/kild/kild-types.ts +++ b/engine/src/kild/kild-types.ts @@ -1,7 +1,7 @@ import type { UiEvent } from './events.ts'; import type { KildCloseEvent } from './hooks.ts'; import type { Inbox } from './inbox.ts'; -import type { KildGitStatus } from './worktree-status.ts'; +import type { BaseSource, KildGitStatus, ResolvedBase } from './worktree-status.ts'; /** * Kild domain — the operator-facing primitive: a set of agents exchanging directed @@ -281,6 +281,11 @@ export interface NewKildSpec { /** Base branch for the worktree + git-status baseline (default: the checkout's current * branch). Editable via `.kild/config.json` `baseBranch` or the `--base` CLI flag. */ base?: string; + /** Where {@link base} came from. Stored with it because the two are one fact: a base + * resolved by fallback and a base a human named produce identical strings, and everything + * measured against them — ahead/behind, a disposal refusal — is only as certain as the + * base is. Recording the string alone is what let a guess be reported as a choice. */ + baseSource?: BaseSource; } /** Lightweight kild descriptor for client lists. Its presence in a `{kilds}` broadcast @@ -496,3 +501,18 @@ export interface CommandAck { requestId: string; result: CommandResult; } + +/** + * A kild's recorded base, read as the pair it is. + * + * The ONE way to turn a stored `base` back into something a git comparison accepts, so no + * caller can hand a stored string to a function expecting a resolved base and have it treated + * as an assertion. That laundering is what made every live kild report its base as `explicit` + * — including the ones whose base was `currentBranch() ?? 'main'` at creation. + */ +export function storedBase(kild: { + base?: string; + baseSource?: BaseSource; +}): ResolvedBase | undefined { + return kild.base ? { base: kild.base, source: kild.baseSource ?? 'unrecorded' } : undefined; +} diff --git a/engine/src/kild/kilds-status.test.ts b/engine/src/kild/kilds-status.test.ts index 71e59e3c..b34da018 100644 --- a/engine/src/kild/kilds-status.test.ts +++ b/engine/src/kild/kilds-status.test.ts @@ -18,6 +18,7 @@ test('formatCompactGitSummary preserves clean known-branch divergence', () => { uncommittedFiles: 0, changedFileCount: 0, conflictsWithBase: null, + baseSource: 'explicit' as const, }), ).toEqual(' · feature-x +2/-1'); }); @@ -34,6 +35,7 @@ test('formatCompactGitSummary renders a null branch as unknown', () => { uncommittedFiles: 0, changedFileCount: 0, conflictsWithBase: null, + baseSource: 'explicit' as const, }), ).toEqual(' · ? +0/-0'); }); @@ -50,6 +52,7 @@ test('formatCompactGitSummary appends dirty and conflict markers', () => { uncommittedFiles: 1, changedFileCount: 1, conflictsWithBase: true, + baseSource: 'explicit' as const, }), ).toEqual(' · feature-x +2/-0 dirty CONFLICTS'); }); @@ -60,11 +63,15 @@ test('a compacted kild carries NO log — the thread is its own cursored resourc id: 'kild-1', name: 'ops', cwd: '/tmp/ops', - agents: [{ handle: 'brain', persona: 'brain' }], + agents: [{ handle: 'brain', ownership: 'owned' as const, persona: 'brain' }], }, ]); expect(compact).toEqual([ - { id: 'kild-1', name: 'ops', agents: [{ handle: 'brain', persona: 'brain' }] }, + { + id: 'kild-1', + name: 'ops', + agents: [{ handle: 'brain', ownership: 'owned' as const, persona: 'brain' }], + }, ]); // Neither the whole log nor a "last couple messages" teaser: a listing that carries messages // is a listing whose size is unbounded in the conversation. @@ -83,13 +90,14 @@ test('git compacts to a summary: changed-file COUNT, not the list (pull discipli uncommittedFiles: 1, changedFiles: ['src/a.ts', 'src/b.ts'], conflictsWithBase: null, + baseSource: 'explicit' as const, }; const compact = compactLiveKilds([ { id: 'kild-1', name: 'ops', cwd: '/tmp/ops', - agents: [{ handle: 'brain', persona: 'brain' }], + agents: [{ handle: 'brain', ownership: 'owned' as const, persona: 'brain' }], git, }, ]); @@ -103,6 +111,7 @@ test('git compacts to a summary: changed-file COUNT, not the list (pull discipli uncommittedFiles: 1, changedFileCount: 2, conflictsWithBase: null, + baseSource: 'explicit' as const, }); // The full list is NOT in the director's compact view. expect(compact[0]?.git).not.toHaveProperty('changedFiles'); @@ -110,7 +119,12 @@ test('git compacts to a summary: changed-file COUNT, not the list (pull discipli test('a live kild without git status has no git key', () => { const compact = compactLiveKilds([ - { id: 'kild-1', name: 'ops', cwd: '/tmp/ops', agents: [{ handle: 'brain', persona: 'brain' }] }, + { + id: 'kild-1', + name: 'ops', + cwd: '/tmp/ops', + agents: [{ handle: 'brain', ownership: 'owned' as const, persona: 'brain' }], + }, ]); expect(compact[0]).not.toHaveProperty('git'); }); @@ -120,7 +134,7 @@ test('collisions: two kilds that touch the same file each name the other', () => id, name, cwd: '/tmp/repo', - agents: [{ handle: 'coder', persona: 'coder' }], + agents: [{ handle: 'coder', ownership: 'owned' as const, persona: 'coder' }], git: { path: `/tmp/${name}`, branch: name, @@ -131,6 +145,7 @@ test('collisions: two kilds that touch the same file each name the other', () => uncommittedFiles: 0, changedFiles, conflictsWithBase: null, + baseSource: 'explicit' as const, }, }); const compact = compactLiveKilds([ @@ -149,22 +164,28 @@ test('per-agent attention + cost ride the compact view, with a kild totals rollu { id: 'kild-1', name: 'ops', + cwd: '/tmp/ops', agents: [ - { handle: 'coder', idle: true, tokens: 3400, cost: 1.25 }, - { handle: 'reviewer', tokens: 600, cost: 0.25 }, + { handle: 'coder', ownership: 'owned' as const, idle: true, tokens: 3400, cost: 1.25 }, + { handle: 'reviewer', ownership: 'owned' as const, tokens: 600, cost: 0.25 }, ], }, ]); expect(compact[0]?.agents).toEqual([ - { handle: 'coder', idle: true, tokens: 3400, cost: 1.25 }, - { handle: 'reviewer', tokens: 600, cost: 0.25 }, + { handle: 'coder', ownership: 'owned' as const, idle: true, tokens: 3400, cost: 1.25 }, + { handle: 'reviewer', ownership: 'owned' as const, tokens: 600, cost: 0.25 }, ]); expect(compact[0]?.totals).toEqual({ tokens: 4000, cost: 1.5 }); }); test('a kild whose agents have no stats gets no totals key', () => { const compact = compactLiveKilds([ - { id: 'kild-1', name: 'ops', cwd: '/tmp/ops', agents: [{ handle: 'coder' }] }, + { + id: 'kild-1', + name: 'ops', + cwd: '/tmp/ops', + agents: [{ handle: 'coder', ownership: 'owned' as const }], + }, ]); expect(compact[0]).not.toHaveProperty('totals'); }); @@ -175,7 +196,7 @@ test('server-computed totals on the live status are preferred over recomputing', id: 'kild-1', name: 'ops', cwd: '/tmp/ops', - agents: [{ handle: 'coder', tokens: 100, cost: 0.1 }], + agents: [{ handle: 'coder', ownership: 'owned' as const, tokens: 100, cost: 0.1 }], totals: { tokens: 4000, cost: 1.5 }, }, ]); @@ -188,7 +209,7 @@ test('compaction copies the agent array without mutating the source kild', () => id: 'kild-1', name: 'ops', cwd: '/tmp/ops', - agents: [{ handle: 'brain', persona: 'brain' }], + agents: [{ handle: 'brain', ownership: 'owned' as const, persona: 'brain' }], }, ]; diff --git a/engine/src/kild/kilds-status.ts b/engine/src/kild/kilds-status.ts index 8ea18f2b..61831168 100644 --- a/engine/src/kild/kilds-status.ts +++ b/engine/src/kild/kilds-status.ts @@ -1,5 +1,5 @@ import { type AgentView, type CostTotals, costTotals, type KildStatus } from './kild-types.ts'; -import type { KildGitStatus } from './worktree-status.ts'; +import type { BaseSource, KildGitStatus } from './worktree-status.ts'; /** The director's compact view of a kild's git state: a summary, not the full * changed-file list. Per the pull-not-push discipline, the director sees a COUNT plus @@ -15,13 +15,23 @@ export interface CompactGitStatus { uncommittedFiles: number; changedFileCount: number; conflictsWithBase: boolean | null; + /** Where {@link base} came from. Declared rather than left riding the spread: it was + * reaching this wire already, typed nowhere and rendered nowhere, which is how a field + * ends up load-bearing by accident. `changedFiles` is dropped here deliberately; this is + * kept deliberately. */ + baseSource: BaseSource; error?: string; } /** One-line git summary shared by kild list and detail displays. */ export function formatCompactGitSummary(git?: CompactGitStatus): string { if (!git) return ''; - return ` · ${git.branch ?? '?'} +${git.ahead}/-${git.behind}${git.dirty ? ' dirty' : ''}${git.conflictsWithBase ? ' CONFLICTS' : ''}`; + // An ahead/behind count is only a fact about somebody's work if the base is. When nobody + // chose the base, the numbers still print — refusing to show them would be worse — but they + // print marked, so a reader knows what the comparison rests on. + const guessed = git.baseSource === 'explicit' || git.baseSource === 'configured'; + const against = guessed ? '' : ` vs ${git.base}?`; + return ` · ${git.branch ?? '?'} +${git.ahead}/-${git.behind}${against}${git.dirty ? ' dirty' : ''}${git.conflictsWithBase ? ' CONFLICTS' : ''}`; } /** One overlap: `kild` also changed `files`. The specific overlapping files ARE the diff --git a/engine/src/kild/memory.test.ts b/engine/src/kild/memory.test.ts index ffdac9a8..fdf351fc 100644 --- a/engine/src/kild/memory.test.ts +++ b/engine/src/kild/memory.test.ts @@ -55,6 +55,7 @@ function kild(cwd: string, overrides: Partial = {}): Kild { to: ['agent'], text: 'Fix the auth bug', ts: 1, + seq: 1, }, { id: 'm1', @@ -63,6 +64,7 @@ function kild(cwd: string, overrides: Partial = {}): Kild { to: ['human'], text: 'second message', ts: 2, + seq: 2, }, ], worktree: 'fix-auth', @@ -170,7 +172,11 @@ test('a RECORDED merge sha wins over inference — the ledger names the commit', test('collectLedgerFacts treats a recorded sha as landed without asking git', async () => { const dir = await repoWithBranch(); // 2 commits ahead of main, nothing merged - const collected = await collectLedgerFacts(dir, 'main', 'abcdef1234567890'); + const collected = await collectLedgerFacts( + dir, + { base: 'main', source: 'explicit' }, + 'abcdef1234567890', + ); expect(collected.landed).toBe(true); expect(collected.landedSha).toBe('abcdef1234567890'); // The code facts are still measured, not overwritten by the land record. @@ -256,7 +262,7 @@ test('collectLedgerFacts reports real commits, files and an unlanded branch', as const dir = await repoWithBranch(); fs.writeFileSync(path.join(dir, 'c.ts'), 'x\n'); // uncommitted - const collected = await collectLedgerFacts(dir, 'main'); + const collected = await collectLedgerFacts(dir, { base: 'main', source: 'explicit' }); expect(collected.gitError).toBeUndefined(); expect(collected.base).toBe('main'); expect(collected.branch).toBe('kild/work'); @@ -273,14 +279,14 @@ test('collectLedgerFacts reports landed once the branch is contained in base', a await git(dir, ['merge', '--no-ff', '-m', 'land', 'kild/work']); await git(dir, ['checkout', 'kild/work']); - const collected = await collectLedgerFacts(dir, 'main'); + const collected = await collectLedgerFacts(dir, { base: 'main', source: 'explicit' }); expect(collected.commits).toBe(0); expect(collected.landed).toBe(true); }); test('collectLedgerFacts on a non-repo yields an error, not a throw', async () => { const dir = fs.mkdtempSync(path.join(tmp, 'norepo-')); - const collected = await collectLedgerFacts(dir, 'main'); + const collected = await collectLedgerFacts(dir, { base: 'main', source: 'explicit' }); expect(collected.gitError).toBeDefined(); expect(collected.landed).toBe(false); expect(collected.commits).toBe(0); diff --git a/engine/src/kild/memory.ts b/engine/src/kild/memory.ts index 22c7e5f6..a64c96ce 100644 --- a/engine/src/kild/memory.ts +++ b/engine/src/kild/memory.ts @@ -4,6 +4,7 @@ import path from 'node:path'; import { kildHome } from './config.ts'; import { reviewCommits } from './git-review.ts'; import type { Kild } from './kild-types.ts'; +import type { ResolvedBase } from './worktree-status.ts'; import { kildGitStatus } from './worktree-status.ts'; /** @@ -115,12 +116,12 @@ export interface KildLedgerFacts { */ export async function collectLedgerFacts( dir: string, - base?: string, + base?: ResolvedBase, landedSha?: string, carried?: { commits: number; files: number }, ): Promise { const status = await kildGitStatus(dir, base); - const review = await reviewCommits(dir, status.base); + const review = await reviewCommits(dir, { base: status.base, source: status.baseSource }); const gitError = status.error ?? review.error; const tip = review.commits[0]?.sha.slice(0, 7); return { diff --git a/engine/src/kild/worktree-status.test.ts b/engine/src/kild/worktree-status.test.ts index 88dff973..7e763c8c 100644 --- a/engine/src/kild/worktree-status.test.ts +++ b/engine/src/kild/worktree-status.test.ts @@ -73,7 +73,7 @@ test('a branch one commit ahead reports ahead 1 and the changed file', async () await git(dir, ['add', '.']); await commit(dir, 'add feature'); - const status = await kildGitStatus(dir, 'main'); + const status = await kildGitStatus(dir, { base: 'main', source: 'explicit' }); expect(status.error).toBeUndefined(); expect(status.branch).toBe('feature'); @@ -88,7 +88,7 @@ test('an uncommitted edit marks the kild dirty', async () => { const dir = await initRepo(); fs.writeFileSync(path.join(dir, 'README.md'), 'changed\n'); - const status = await kildGitStatus(dir, 'main'); + const status = await kildGitStatus(dir, { base: 'main', source: 'explicit' }); expect(status.error).toBeUndefined(); expect(status.dirty).toBe(true); @@ -119,7 +119,7 @@ test('a branch that merges cleanly into base reports conflictsWithBase false', a await git(dir, ['add', '.']); await commit(dir, 'add feature'); // new file, no overlap with base - const status = await kildGitStatus(dir, 'main'); + const status = await kildGitStatus(dir, { base: 'main', source: 'explicit' }); expect(status.error).toBeUndefined(); expect(status.ahead).toBe(1); @@ -139,7 +139,7 @@ test('a branch that edits the same line as base reports conflictsWithBase true', await commit(dir, 'main edit'); await git(dir, ['checkout', 'feature']); - const status = await kildGitStatus(dir, 'main'); + const status = await kildGitStatus(dir, { base: 'main', source: 'explicit' }); expect(status.ahead).toBe(1); expect(status.behind).toBe(1); @@ -149,7 +149,7 @@ test('a branch that edits the same line as base reports conflictsWithBase true', test('a missing base ref is reported as an error, not a crash', async () => { const dir = await initRepo(); - const status = await kildGitStatus(dir, 'does-not-exist'); + const status = await kildGitStatus(dir, { base: 'does-not-exist', source: 'explicit' }); expect(status.error).toBeDefined(); expect(status.base).toBe('does-not-exist'); diff --git a/engine/src/kild/worktree-status.ts b/engine/src/kild/worktree-status.ts index a5674747..599fa70a 100644 --- a/engine/src/kild/worktree-status.ts +++ b/engine/src/kild/worktree-status.ts @@ -17,6 +17,10 @@ export interface KildGitStatus { path: string; // the dir inspected branch: string | null; base: string; // base branch compared against (default: main) + /** Where {@link base} came from. `explicit` is an assertion by the caller; the others are + * the engine's own resolution, and a client that renders ahead/behind should say so — an + * ahead-count measured against a guessed base is not a fact about anybody's work. */ + baseSource: BaseSource; ahead: number; // commits on branch not in base behind: number; // commits on base not in branch dirty: boolean; // uncommitted changes present @@ -38,28 +42,68 @@ async function runGit(dir: string, args: string[]): Promise { } } +/** + * Where a base branch came from. Carried alongside the branch itself because a GUESS and a + * CONFIGURED FACT are different claims, and everything downstream — a git summary, a disposal + * refusal — reads as authoritative unless told otherwise. + * + * This is not hypothetical. 116 worktrees sat unreclaimable behind a refusal reading "carries + * 365 commits not in main", which was true and meaningless: that repo's default is `dev`, so + * every tree was hundreds of commits ahead of a branch it had never forked from, by + * construction. The refusal named the base. What it could not say was that nobody had chosen + * it. Four measurement passes went into rediscovering that, and 5.4 GB went into not knowing. + */ +export type BaseSource = + /** A caller named it — `--base`, or a query param. The only one that is an assertion. */ + | 'explicit' + /** `baseBranch` in project or global config. Chosen deliberately, just not in this call. */ + | 'configured' + /** The checkout's current branch at creation time. Plausible, nobody chose it. */ + | 'current-branch' + /** `origin/HEAD` — the remote's default AS CACHED LOCALLY. Git writes this symref at clone + * time and never refreshes it, so it goes stale silently when the remote's default moves. + * Better than a literal guess; still not a fact anyone asserted. */ + | 'origin-head' + /** Nothing said otherwise, so `main`. A pure guess, and the one that cost the 116. */ + | 'fallback' + /** A kild recorded before provenance was tracked. Not a guess and not a choice — an + * absence. Named rather than folded into `fallback`, because claiming we know it was + * guessed is the same overreach as claiming we know it was chosen. */ + | 'unrecorded'; + +export interface ResolvedBase { + base: string; + source: BaseSource; +} + /** The base branch to compare against when the caller doesn't name one: the remote's - * default (`origin/HEAD`, minus the `origin/` prefix) if set, else `main`. Shared - * with git-review so summary status and review drill-down agree on the baseline. */ -export async function resolveDefaultBase(dir: string): Promise { + * default (`origin/HEAD`, minus the `origin/` prefix) if set, else `main` — and WHICH of + * those it was. Shared with git-review so summary status and review drill-down agree. */ +export async function resolveDefaultBase(dir: string): Promise { const head = await runGit(dir, ['symbolic-ref', '--short', 'refs/remotes/origin/HEAD']); if (head.ok) { const branch = head.stdout.trim().replace(/^origin\//, ''); - if (branch) return branch; + if (branch) return { base: branch, source: 'origin-head' }; } - return 'main'; + return { base: 'main', source: 'fallback' }; } /** Inspect one kild directory's git state relative to `base` (default: the * remote default branch, else `main`). Never throws: a non-git dir, a missing base * ref, or any git error returns a well-formed object with `error` set and safe * defaults so a driving agent can surface the state without crashing. */ -export async function kildGitStatus(dir: string, base?: string): Promise { - const resolvedBase = base ?? (await resolveDefaultBase(dir)); +export async function kildGitStatus(dir: string, base?: ResolvedBase): Promise { + // Takes a ResolvedBase, never a bare string. A string parameter meant any caller holding a + // stored base passed it as though a human had named it, so `Kild.base` — itself resolved by + // a fallback chain at creation — was reported as `explicit` for every live kild. The base and + // where it came from travel together or the provenance is decoration. + const resolved = base ?? (await resolveDefaultBase(dir)); + const resolvedBase = resolved.base; const status: KildGitStatus = { path: dir, branch: null, base: resolvedBase, + baseSource: resolved.source, ahead: 0, behind: 0, dirty: false, diff --git a/engine/src/kild/worktree.ts b/engine/src/kild/worktree.ts index 9aa7bbe6..0e9bd53a 100644 --- a/engine/src/kild/worktree.ts +++ b/engine/src/kild/worktree.ts @@ -4,6 +4,7 @@ import path from 'node:path'; import { promisify } from 'node:util'; import { configuredBaseBranch, kildHome } from './config.ts'; +import type { ResolvedBase } from './worktree-status.ts'; // execFile (no shell) + a branch-name allowlist: the brain's create_worktree tool // and UI clients' worktree selectors feed a (possibly LLM-generated) name in here, @@ -82,12 +83,25 @@ export async function currentBranch(repo: string): Promise { return branch || undefined; } -/** Resolve the base branch for a worktree/kild in `cwd`: explicit `flag` wins, else the - * configured `baseBranch` (project over global), else the checkout's current branch, else - * `main`. This is the branch new worktrees fork from and that git status is measured - * against, so ahead/behind + collisions reflect this kild's own work. */ -export async function resolveBaseBranch(cwd: string, flag?: string): Promise { - return flag ?? (await configuredBaseBranch(cwd)) ?? (await currentBranch(cwd)) ?? 'main'; +/** + * Resolve the base branch for a worktree/kild in `cwd`, AND say where it came from: explicit + * `flag` wins, else the configured `baseBranch` (project over global), else the checkout's + * current branch, else `main`. + * + * The provenance is not decoration. This result is stored as `Kild.base` at creation and then + * handed to every git comparison for the rest of the kild's life. Returned as a bare string it + * was indistinguishable from a base a human named, so a guess laundered into an assertion the + * moment it was stored — and every refusal measured against it claimed a certainty nobody had. + * That is the failure that held 116 worktrees; storing the string without its source is how it + * would have survived the fix. + */ +export async function resolveBaseBranch(cwd: string, flag?: string): Promise { + if (flag) return { base: flag, source: 'explicit' }; + const configured = await configuredBaseBranch(cwd); + if (configured) return { base: configured, source: 'configured' }; + const current = await currentBranch(cwd); + if (current) return { base: current, source: 'current-branch' }; + return { base: 'main', source: 'fallback' }; } /** Create a fresh isolated worktree on a `kild/` branch, force-resetting any diff --git a/engine/src/server.ts b/engine/src/server.ts index eff13b4d..685cc5e3 100644 --- a/engine/src/server.ts +++ b/engine/src/server.ts @@ -20,6 +20,7 @@ import { landMerge, landPlan } from './kild/kild-land.ts'; import { kildManager } from './kild/kild-manager.ts'; import { type KildTree, kildTrees, orphanTrees } from './kild/kild-trees.ts'; import type { AgentSpec, CommandResult, KildIdentity, KildStatus } from './kild/kild-types.ts'; +import { storedBase } from './kild/kild-types.ts'; import { listPersonas } from './kild/personas.ts'; import { findProject, loadProjects } from './kild/projects.ts'; import { @@ -465,7 +466,7 @@ app.get('/api/kilds/:id', async (c) => { path: target.value.dir, repo: target.value.repo, }), - git: await kildGitStatus(target.value.dir, target.value.base), + git: await kildGitStatus(target.value.dir, storedBase(target.value)), }); }); @@ -829,7 +830,7 @@ app.delete('/api/kilds/:id', async (c) => { if (!target.ok) { return c.json({ error: target.message, code: target.code }, kildResultStatus(target)); } - const { worktree, repo, dir, base, live, name } = target.value; + const { worktree, repo, dir, live, name } = target.value; if (!worktree) { return c.json( { @@ -844,7 +845,7 @@ app.delete('/api/kilds/:id', async (c) => { repo, dir, branch: `kild/${worktree}`, - base, + base: storedBase(target.value), inUse: worktreesInUse().has(worktree), force, }); @@ -902,7 +903,7 @@ app.get('/api/kilds/:id/land', async (c) => { if (!target.ok) { return c.json({ error: target.message, code: target.code }, kildResultStatus(target)); } - return c.json({ ...(await landPlan(target.value.dir, target.value.base)), dryRun: true }); + return c.json({ ...(await landPlan(target.value.dir, storedBase(target.value))), dryRun: true }); }); app.post('/api/kilds/:id/land', async (c) => { const id = c.req.param('id'); @@ -910,7 +911,7 @@ app.post('/api/kilds/:id/land', async (c) => { if (!target.ok) { return c.json({ error: target.message, code: target.code }, kildResultStatus(target)); } - const result = await landMerge(target.value.repo, target.value.dir, target.value.base); + const result = await landMerge(target.value.repo, target.value.dir, storedBase(target.value)); // A land that did not happen is NOT a success — the caller must be able to tell. if (!result.merged) return c.json({ ...result, dryRun: false }, 409); if (target.value.live && result.sha) { @@ -933,14 +934,14 @@ app.get('/api/kilds/:id/git/commits', async (c) => { if (!located.ok) { return c.json({ error: located.message, code: located.code }, kildResultStatus(located)); } - return c.json(await reviewCommits(located.value.dir, located.value.base)); + return c.json(await reviewCommits(located.value.dir, storedBase(located.value))); }); app.get('/api/kilds/:id/git/files', async (c) => { const located = kildManager.kildDir(c.req.param('id')); if (!located.ok) { return c.json({ error: located.message, code: located.code }, kildResultStatus(located)); } - return c.json(await reviewFiles(located.value.dir, located.value.base)); + return c.json(await reviewFiles(located.value.dir, storedBase(located.value))); }); app.get('/api/kilds/:id/git/diff', async (c) => { const file = c.req.query('path'); @@ -949,7 +950,7 @@ app.get('/api/kilds/:id/git/diff', async (c) => { if (!located.ok) { return c.json({ error: located.message, code: located.code }, kildResultStatus(located)); } - const diff = await reviewDiff(located.value.dir, located.value.base, file); + const diff = await reviewDiff(located.value.dir, storedBase(located.value), file); // The traversal guard: only a path git itself reported may be diffed. if (diff.unknownPath) return c.json({ error: diff.error }, 404); return c.json(diff); diff --git a/engine/tsconfig.json b/engine/tsconfig.json index 6813f48c..03fc9cfc 100644 --- a/engine/tsconfig.json +++ b/engine/tsconfig.json @@ -16,8 +16,8 @@ "verbatimModuleSyntax": true, "forceConsistentCasingInFileNames": true, "noEmit": true, - "types": ["node"] + "types": ["node", "bun"] }, "include": ["src/**/*.ts"], - "exclude": ["dist", "node_modules", "src/**/*.test.ts"] + "exclude": ["dist", "node_modules"] }