diff --git a/package-lock.json b/package-lock.json index ec6a2403..7411c76d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -8457,7 +8457,7 @@ }, "packages/api": { "name": "@diffity/api", - "version": "0.10.9", + "version": "0.10.10", "dependencies": { "@diffity/parser": "*" }, @@ -8468,7 +8468,7 @@ }, "packages/cli": { "name": "@naturalcycles/diffity", - "version": "0.10.9", + "version": "0.10.10", "license": "MIT", "dependencies": { "commander": "^14.0.3", @@ -8492,7 +8492,7 @@ }, "packages/git": { "name": "@diffity/git", - "version": "0.10.9", + "version": "0.10.10", "devDependencies": { "@types/node": "^25.5.0", "typescript": "^5.9.3", @@ -8501,7 +8501,7 @@ }, "packages/github": { "name": "@diffity/github", - "version": "0.10.9", + "version": "0.10.10", "dependencies": { "@diffity/api": "*", "@diffity/parser": "*" @@ -8514,7 +8514,7 @@ }, "packages/parser": { "name": "@diffity/parser", - "version": "0.10.9", + "version": "0.10.10", "devDependencies": { "typescript": "^5.9.3", "vitest": "^4.1.0" @@ -8522,7 +8522,7 @@ }, "packages/ui": { "name": "@diffity/ui", - "version": "0.10.9", + "version": "0.10.10", "dependencies": { "@diffity/api": "*", "@diffity/parser": "*", diff --git a/packages/api/package.json b/packages/api/package.json index cc66cfec..f785f821 100644 --- a/packages/api/package.json +++ b/packages/api/package.json @@ -1,6 +1,6 @@ { "name": "@diffity/api", - "version": "0.10.9", + "version": "0.10.10", "private": true, "type": "module", "main": "./dist/index.js", diff --git a/packages/cli/package.json b/packages/cli/package.json index 025e0c3d..0f9df398 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -1,6 +1,6 @@ { "name": "@naturalcycles/diffity", - "version": "0.10.9", + "version": "0.10.10", "description": "Agent-agnostic, GitHub-style diff viewer and code review tool with a live agent loop", "type": "module", "bin": { diff --git a/packages/cli/src/inbox/daemon.ts b/packages/cli/src/inbox/daemon.ts index 92842357..905d5261 100644 --- a/packages/cli/src/inbox/daemon.ts +++ b/packages/cli/src/inbox/daemon.ts @@ -1,4 +1,4 @@ -import { createServer, type Server } from 'node:http'; +import { createServer, type Server, type ServerResponse } from 'node:http'; import { existsSync, readdirSync, readFileSync, rmSync } from 'node:fs'; import { basename, join } from 'node:path'; import { getViewerLogin, searchReviewRequested, viewPr } from '@diffity/github'; @@ -10,6 +10,9 @@ import { removeWorktree, cloneDir } from './worktree.js'; import { InboxStore } from './store.js'; import { runTick, type Forge } from './tick.js'; import { buildView } from './view.js'; +import { resolveOpen } from './open.js'; +import { openPreparedSession, realOpenSessionDeps, type OpenSessionDeps } from './open-session.js'; +import { inboxPage } from './page.js'; const realForge: Forge = { viewerLogin: getViewerLogin, @@ -32,6 +35,8 @@ export interface DaemonOptions { once?: boolean; /** The forge to poll; defaults to the real GitHub one. Overridden only by tests. */ forge?: Forge; + /** How a prepared review is brought up as a session; defaults to the real one. Tests override it. */ + openDeps?: OpenSessionDeps; } /** @@ -85,7 +90,8 @@ export async function runDaemon( // Bind the port first: it is the daemon's singleton lock, so a second daemon exits here (via the // server's error handler) before it can reclaim and kill the first one's in-flight servers. - const server = await bindInboxServer(store, config, log); + const openDeps = options.openDeps ?? realOpenSessionDeps(nodePath, entry); + const server = await bindInboxServer(store, config, log, openDeps); reclaimLeftoverServers(log); const timer = setInterval(() => void tick(), config.pollMinutes * 60_000); void tick(); @@ -135,17 +141,64 @@ function reclaimLeftoverServers(log: (message: string) => void): void { } } -export function startInboxServer(store: InboxStore, config: InboxConfig, log: (message: string) => void): Server { +export function startInboxServer(store: InboxStore, config: InboxConfig, log: (message: string) => void, openDeps: OpenSessionDeps): Server { const server = createServer((req, res) => { - const openBase = `http://localhost:${config.port}`; - if (req.method === 'GET' && (req.url === '/api/inbox' || req.url === '/api/inbox/')) { - const view = buildView(store, openBase, new Date().toISOString()); - res.writeHead(200, { 'Content-Type': 'application/json' }); - res.end(JSON.stringify(view)); - return; + // The whole handler is guarded: an unhandled throw here (a malformed percent-escape, say) would + // otherwise have no catch and take the long-running daemon down with it. + try { + // Loopback binding is not enough on its own: a page on another site can rebind its own + // hostname to 127.0.0.1, so a stranger's Host header must not reach the reviewer's PR list. + // Judged against the connection's own port, which is the port actually bound. + if (!isLocalHost(req.headers.host, req.socket.localPort)) { + res.writeHead(403, { 'Content-Type': 'text/plain; charset=utf-8' }); + res.end('forbidden'); + return; + } + + // The host the reader actually used — localhost or 127.0.0.1, already checked — so the links + // on the page point back at the same origin and a click on them is not cross-site. + const openBase = `http://${req.headers.host}`; + const url = req.url ?? '/'; + + if (req.method === 'GET' && (url === '/' || url === '/index.html')) { + res.writeHead(200, { 'Content-Type': 'text/html; charset=utf-8' }); + res.end(inboxPage()); + return; + } + if (req.method === 'GET' && (url === '/api/inbox' || url === '/api/inbox/')) { + const view = buildView(store, openBase, new Date().toISOString()); + res.writeHead(200, { 'Content-Type': 'application/json' }); + res.end(JSON.stringify(view)); + return; + } + if (req.method === 'GET' && url.startsWith('/open/')) { + // A state-changing GET, so a cross-site fetch — a drive-by trying to spawn a session — is + // refused; a click from the inbox page itself is same-origin, and a direct navigation none. + if (req.headers['sec-fetch-site'] === 'cross-site') { + res.writeHead(403, { 'Content-Type': 'text/plain; charset=utf-8' }); + res.end('forbidden'); + return; + } + let id: string; + try { + id = decodeURIComponent(url.slice('/open/'.length)); + } catch { + res.writeHead(400, { 'Content-Type': 'text/plain; charset=utf-8' }); + res.end('bad request'); + return; + } + void handleOpen(store, id, openDeps, log, res).catch(err => log(`open failed: ${err instanceof Error ? err.message : err}`)); + return; + } + res.writeHead(404, { 'Content-Type': 'application/json' }); + res.end(JSON.stringify({ error: 'not found' })); + } catch (err) { + log(`request handler error: ${err instanceof Error ? err.message : err}`); + if (!res.headersSent) { + res.writeHead(500, { 'Content-Type': 'text/plain; charset=utf-8' }); + } + res.end('internal error'); } - res.writeHead(404, { 'Content-Type': 'application/json' }); - res.end(JSON.stringify({ error: 'not found' })); }); server.on('error', err => { const code = (err as NodeJS.ErrnoException).code; @@ -159,8 +212,38 @@ export function startInboxServer(store: InboxStore, config: InboxConfig, log: (m return server; } +/** Brings a prepared review up as a live session and redirects the browser to it. */ +async function handleOpen(store: InboxStore, id: string, openDeps: OpenSessionDeps, log: (message: string) => void, res: ServerResponse): Promise { + try { + const resolution = resolveOpen(store, id); + if (!resolution.ok) { + res.writeHead(resolution.status, { 'Content-Type': 'text/plain; charset=utf-8' }); + res.end(resolution.message); + return; + } + log(`opening ${id}`); + const { url, imported, importError } = await openPreparedSession(resolution.pr.worktreePath!, resolution.pr.bundlePath!, openDeps); + if (!imported) { + log(`opened ${id} but its findings did not import: ${importError}`); + } + res.writeHead(302, { Location: url }); + res.end(); + } catch (err) { + log(`could not open ${id}: ${err instanceof Error ? err.message : err}`); + if (!res.headersSent) { + res.writeHead(500, { 'Content-Type': 'text/plain; charset=utf-8' }); + res.end(`Could not open ${id}: ${err instanceof Error ? err.message : err}`); + } + } +} + +/** A request whose Host is this loopback server's own address (localhost or 127.0.0.1, right port). */ +function isLocalHost(host: string | undefined, port: number | undefined): boolean { + return port != null && (host === `localhost:${port}` || host === `127.0.0.1:${port}`); +} + /** Resolves once the port is held; a clash exits through the server's own error handler first. */ -function bindInboxServer(store: InboxStore, config: InboxConfig, log: (message: string) => void): Promise { - const server = startInboxServer(store, config, log); +function bindInboxServer(store: InboxStore, config: InboxConfig, log: (message: string) => void, openDeps: OpenSessionDeps): Promise { + const server = startInboxServer(store, config, log, openDeps); return new Promise(resolve => server.once('listening', () => resolve(server))); } diff --git a/packages/cli/src/inbox/open-session.ts b/packages/cli/src/inbox/open-session.ts new file mode 100644 index 00000000..3910883f --- /dev/null +++ b/packages/cli/src/inbox/open-session.ts @@ -0,0 +1,96 @@ +import { spawn, execFileSync } from 'node:child_process'; +import { readFileSync, realpathSync } from 'node:fs'; +import { createHash } from 'node:crypto'; +import { checkInstanceHealth, findInstanceForRepo } from '../registry.js'; + +/** + * Brings a prepared review up as a live diffity session the reviewer can open: a server over the + * worktree, diffing against the pull request's base, with the prepared findings imported. Returns + * the URL to send the browser to. Import failure is not fatal — the diff is still worth opening — + * but it is surfaced to the caller. + */ +export async function openPreparedSession(worktree: string, bundlePath: string, deps: OpenSessionDeps): Promise { + const ref = deps.baseRefOf(bundlePath); + const port = await deps.ensureServer(worktree, ref); + try { + deps.importBundle(worktree, bundlePath); + } catch (err) { + return { url: `http://localhost:${port}/`, imported: false, importError: err instanceof Error ? err.message : String(err) }; + } + return { url: `http://localhost:${port}/`, imported: true }; +} + +/** The base commit a bundle was built against; the session diffs the worktree against it. */ +export function baseRefOf(bundlePath: string): string { + const bundle = JSON.parse(readFileSync(bundlePath, 'utf-8')) as { baseSha?: string | null }; + if (!bundle.baseSha) { + throw new Error(`The bundle at ${bundlePath} records no base commit.`); + } + return bundle.baseSha; +} + +/** + * The real `ensureServer`: a healthy diffity already serving this worktree is reused, otherwise one + * is started in the reviewer's own diffity (no data-dir override), so it shows up in `diffity list` + * and behaves like any session they opened themselves. + */ +export function realOpenSessionDeps(nodePath: string, entry: string): OpenSessionDeps { + return { + baseRefOf, + ensureServer: (worktree, ref) => ensureServer(nodePath, entry, worktree, ref), + importBundle: (worktree, bundlePath) => { + execFileSync(nodePath, [entry, '--repo', worktree, 'agent', 'import-bundle', bundlePath], { stdio: 'pipe' }); + }, + }; +} + +export async function ensureServer(nodePath: string, entry: string, worktree: string, ref: string, waitMs = 30_000): Promise { + const hash = repoHash(worktree); + // A healthy server already on this worktree is reused as-is. The worktree lives under the inbox's + // own directory and is only ever served at the pull request's base, so its ref is the one wanted. + const existing = findInstanceForRepo(hash); + if (existing && await checkInstanceHealth(existing.port)) { + return existing.port; + } + + const child = spawn(nodePath, [entry, '--repo', worktree, '--no-open', '--quiet', ref], { detached: true, stdio: 'ignore' }); + child.unref(); + + const deadline = Date.now() + waitMs; + while (Date.now() < deadline) { + await sleep(400); + const entryRow = findInstanceForRepo(hash); + if (entryRow && await checkInstanceHealth(entryRow.port)) { + return entryRow.port; + } + } + try { if (child.pid) process.kill(child.pid, 'SIGTERM'); } catch { /* already gone */ } + throw new Error(`diffity did not start for ${worktree} within ${waitMs / 1000}s`); +} + +/** The server registers under the hash of its resolved repo root, so resolve symlinks before hashing. */ +export function repoHash(worktree: string): string { + let root = worktree; + try { root = realpathSync(worktree); } catch { /* not yet on disk; hash the path as given */ } + return createHash('sha256').update(root).digest('hex').slice(0, 12); +} + +function sleep(ms: number): Promise { + return new Promise(resolve => setTimeout(resolve, ms)); +} + +export interface OpenSessionDeps { + /** Reads the base ref recorded in the bundle, so the session diffs the same change. */ + baseRefOf(bundlePath: string): string; + /** Ensures a diffity server for the worktree at that ref and returns its port. */ + ensureServer(worktree: string, ref: string): Promise; + /** Adds the prepared review's threads and tours to the running session. */ + importBundle(worktree: string, bundlePath: string): void; +} + +export interface OpenedSession { + url: string; + imported: boolean; + /** Why the import did not happen, when it didn't; the diff is still opened. */ + importError?: string; +} diff --git a/packages/cli/src/inbox/open.ts b/packages/cli/src/inbox/open.ts new file mode 100644 index 00000000..b5ba547e --- /dev/null +++ b/packages/cli/src/inbox/open.ts @@ -0,0 +1,24 @@ +import type { InboxPr, InboxStore } from './store.js'; + +export type OpenResolution = + | { ok: true; pr: InboxPr } + | { ok: false; status: number; message: string }; + +/** + * Whether a pull request can be opened, and why not when it can't. Only a prepared (or stale but + * still prepared) review has a worktree and a bundle to open; a queued, skipped or failed one has + * nothing to show yet. + */ +export function resolveOpen(store: InboxStore, id: string): OpenResolution { + const pr = store.get(id); + if (!pr) { + return { ok: false, status: 404, message: `No pull request ${id} in the inbox.` }; + } + if (pr.status !== 'prepared' && pr.status !== 'stale') { + return { ok: false, status: 409, message: `${id} is ${pr.status}, not ready to open.` }; + } + if (!pr.worktreePath || !pr.bundlePath) { + return { ok: false, status: 409, message: `${id} has no prepared worktree to open.` }; + } + return { ok: true, pr }; +} diff --git a/packages/cli/src/inbox/page.ts b/packages/cli/src/inbox/page.ts new file mode 100644 index 00000000..ae892234 --- /dev/null +++ b/packages/cli/src/inbox/page.ts @@ -0,0 +1,143 @@ +/** + * The inbox page, served at `/`. Self-contained (no build step, no external requests): it polls + * `/api/inbox` and renders the three groups, opening a prepared review in a new tab via `/open/:id`. + */ +export function inboxPage(): string { + return ` + + + + +diffity inbox + + + +
+

diffity inbox

+ loading… +
+
+ + + + + +
+ + +`; +} diff --git a/packages/cli/tests/inbox-open.test.ts b/packages/cli/tests/inbox-open.test.ts new file mode 100644 index 00000000..93374037 --- /dev/null +++ b/packages/cli/tests/inbox-open.test.ts @@ -0,0 +1,247 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { request } from 'node:http'; +import { execFileSync } from 'node:child_process'; +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { dirname, join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { tmpdir } from 'node:os'; +import { resolveOpen } from '../src/inbox/open.js'; +import { openPreparedSession, baseRefOf, ensureServer, repoHash, type OpenSessionDeps } from '../src/inbox/open-session.js'; +import { startInboxServer } from '../src/inbox/daemon.js'; +import { InboxStore } from '../src/inbox/store.js'; +import { readRegistry } from '../src/registry.js'; +import type { PrSnapshot } from '@diffity/github'; + +const ENTRY = join(dirname(fileURLToPath(import.meta.url)), '..', 'dist', 'index.js'); + +let root: string; + +function snapshot(): PrSnapshot { + return { + owner: 'o', repo: 'r', number: 4, title: 'A change', url: 'https://github.com/o/r/pull/4', + author: 'alice', isBot: false, isDraft: false, state: 'OPEN', headSha: 'aaa', baseRef: 'main', + additions: 3, deletions: 1, changedFiles: 2, updatedAt: 'now', + }; +} + +function preparedStore(): InboxStore { + const store = new InboxStore(':memory:'); + store.observe(snapshot(), true, 'now'); + store.markPrepared('o/r#4', { headSha: 'aaa', bundlePath: '/b.json', worktreePath: '/wt', logPath: '/l', at: 'now' }); + return store; +} + +beforeEach(() => { root = mkdtempSync(join(tmpdir(), 'diffity-open-')); }); +afterEach(() => { rmSync(root, { recursive: true, force: true }); }); + +/** A GET with forged headers fetch() will not set, resolving to the status code. */ +function rawGet(port: number, path: string, headers: Record): Promise { + return new Promise((resolve, reject) => { + const req = request({ host: '127.0.0.1', port, path, method: 'GET', headers }, res => { + res.resume(); + resolve(res.statusCode ?? 0); + }); + req.on('error', reject); + req.end(); + }); +} + +describe('resolveOpen', () => { + it('accepts a prepared pull request with a worktree and bundle', () => { + const store = preparedStore(); + const r = resolveOpen(store, 'o/r#4'); + expect(r.ok).toBe(true); + store.close(); + }); + + it('refuses an unknown, a not-ready, or a worktree-less pull request', () => { + const store = preparedStore(); + expect(resolveOpen(store, 'o/r#9')).toMatchObject({ ok: false, status: 404 }); + + store.observe({ ...snapshot(), number: 5 }, true, 'now'); + store.setStatus('o/r#5', 'queued'); + expect(resolveOpen(store, 'o/r#5')).toMatchObject({ ok: false, status: 409 }); + store.close(); + }); +}); + +describe('baseRefOf', () => { + it('reads the base commit from a bundle and refuses one without it', () => { + const withBase = join(root, 'a.json'); + writeFileSync(withBase, JSON.stringify({ baseSha: 'b'.repeat(40) })); + expect(baseRefOf(withBase)).toBe('b'.repeat(40)); + + const noBase = join(root, 'b.json'); + writeFileSync(noBase, JSON.stringify({ baseSha: null })); + expect(() => baseRefOf(noBase)).toThrow(/no base commit/); + }); +}); + +describe('openPreparedSession', () => { + it('starts the session at the base and imports the bundle, returning its url', async () => { + const calls: string[] = []; + const deps: OpenSessionDeps = { + baseRefOf: () => 'basesha', + ensureServer: (wt, ref) => { calls.push(`ensure ${wt} ${ref}`); return Promise.resolve(5599); }, + importBundle: (wt, bundle) => { calls.push(`import ${wt} ${bundle}`); }, + }; + + const result = await openPreparedSession('/wt', '/b.json', deps); + + expect(result).toEqual({ url: 'http://localhost:5599/', imported: true }); + expect(calls).toEqual(['ensure /wt basesha', 'import /wt /b.json']); + }); + + it('still opens the diff when the import fails, flagging it', async () => { + const deps: OpenSessionDeps = { + baseRefOf: () => 'basesha', + ensureServer: () => Promise.resolve(5599), + importBundle: () => { throw new Error('head moved'); }, + }; + const result = await openPreparedSession('/wt', '/b.json', deps); + expect(result).toEqual({ url: 'http://localhost:5599/', imported: false, importError: 'head moved' }); + }); +}); + +describe('the real ensureServer', () => { + it('hashes a worktree the same way the diffity server it starts registers it', async () => { + const prev = process.env.DIFFITY_DATA_DIR; + process.env.DIFFITY_DATA_DIR = join(root, 'data'); + const repo = join(root, 'repo'); + execFileSync('git', ['init', '-b', 'main', repo], { stdio: 'pipe' }); + execFileSync('git', ['config', 'user.email', 't@t'], { cwd: repo, stdio: 'pipe' }); + execFileSync('git', ['config', 'user.name', 'T'], { cwd: repo, stdio: 'pipe' }); + writeFileSync(join(repo, 'a.ts'), 'const a = 1;\n'); + execFileSync('git', ['add', '.'], { cwd: repo, stdio: 'pipe' }); + execFileSync('git', ['commit', '-m', 'init'], { cwd: repo, stdio: 'pipe' }); + + let port = 0; + try { + port = await ensureServer(process.execPath, ENTRY, repo, 'work', 20_000); + // The entry the server registered must carry the hash open-session looks it up by. + const entry = readRegistry().find(e => e.port === port); + expect(entry).toBeDefined(); + expect(entry!.repoHash).toBe(repoHash(repo)); + } finally { + const entry = readRegistry().find(e => e.port === port); + if (entry) { try { process.kill(entry.pid, 'SIGKILL'); } catch { /* gone */ } } + if (prev === undefined) delete process.env.DIFFITY_DATA_DIR; else process.env.DIFFITY_DATA_DIR = prev; + } + }, 30_000); + + it('throws when nothing registers before the deadline', async () => { + const prev = process.env.DIFFITY_DATA_DIR; + process.env.DIFFITY_DATA_DIR = join(root, 'empty-data'); + const idle = join(root, 'idle.mjs'); + writeFileSync(idle, 'setInterval(() => {}, 1000);\n'); + try { + await expect(ensureServer(process.execPath, idle, join(root, 'wt'), 'work', 800)).rejects.toThrow(/did not start/); + } finally { + if (prev === undefined) delete process.env.DIFFITY_DATA_DIR; else process.env.DIFFITY_DATA_DIR = prev; + } + }, 10_000); +}); + +describe('the inbox server routes', () => { + const stubOpen: OpenSessionDeps = { + baseRefOf: () => 'basesha', + ensureServer: () => Promise.resolve(7788), + importBundle: () => {}, + }; + + async function serve(store: InboxStore) { + const config = { pollMinutes: 5, port: 0, reposDir: root, worktreesDir: root, filter: '', prepare: ['x'], prepareTimeoutMinutes: 30 }; + const server = startInboxServer(store, config, () => {}, stubOpen); + await new Promise(resolve => server.on('listening', resolve)); + const { port } = server.address() as { port: number }; + return { port, server }; + } + + it('serves the page at / and the view at /api/inbox', async () => { + const store = preparedStore(); + const { port, server } = await serve(store); + try { + const page = await fetch(`http://127.0.0.1:${port}/`); + expect(page.headers.get('content-type')).toContain('text/html'); + expect(await page.text()).toContain('diffity inbox'); + + const api = await (await fetch(`http://127.0.0.1:${port}/api/inbox`)).json(); + expect(api.ready[0].id).toBe('o/r#4'); + } finally { + server.close(); + store.close(); + } + }); + + it('redirects /open/ to the opened session, and 409s a not-ready one', async () => { + const store = preparedStore(); + const { port, server } = await serve(store); + try { + const res = await fetch(`http://127.0.0.1:${port}/open/${encodeURIComponent('o/r#4')}`, { redirect: 'manual' }); + expect(res.status).toBe(302); + expect(res.headers.get('location')).toBe('http://localhost:7788/'); + + store.observe({ ...snapshot(), number: 8 }, true, 'now'); + const notReady = await fetch(`http://127.0.0.1:${port}/open/${encodeURIComponent('o/r#8')}`, { redirect: 'manual' }); + expect(notReady.status).toBe(409); + } finally { + server.close(); + store.close(); + } + }); + + it('answers a malformed /open URL with 400 and keeps running', async () => { + const store = preparedStore(); + const { port, server } = await serve(store); + try { + const bad = await fetch(`http://127.0.0.1:${port}/open/%`); + expect(bad.status).toBe(400); + // The daemon is still up and serving afterwards. + const ok = await fetch(`http://127.0.0.1:${port}/api/inbox`); + expect(ok.status).toBe(200); + } finally { + server.close(); + store.close(); + } + }); + + it('rejects a foreign Host header and a cross-site open', async () => { + const store = preparedStore(); + const { port, server } = await serve(store); + try { + // fetch() forbids setting Host and Sec-Fetch-*, so a raw request is needed to forge them. + const rebind = await rawGet(port, '/api/inbox', { Host: 'evil.example.com:1234' }); + expect(rebind).toBe(403); + + const driveBy = await rawGet(port, `/open/${encodeURIComponent('o/r#4')}`, { + Host: `127.0.0.1:${port}`, 'Sec-Fetch-Site': 'cross-site', + }); + expect(driveBy).toBe(403); + } finally { + server.close(); + store.close(); + } + }); + + it('surfaces an import failure in the log but still redirects', async () => { + const store = preparedStore(); + const logs: string[] = []; + const failingOpen: OpenSessionDeps = { + baseRefOf: () => 'basesha', + ensureServer: () => Promise.resolve(7788), + importBundle: () => { throw new Error('head moved'); }, + }; + const config = { pollMinutes: 5, port: 0, reposDir: root, worktreesDir: root, filter: '', prepare: ['x'], prepareTimeoutMinutes: 30 }; + const server = startInboxServer(store, config, m => logs.push(m), failingOpen); + await new Promise(resolve => server.on('listening', resolve)); + const { port } = server.address() as { port: number }; + try { + const res = await fetch(`http://127.0.0.1:${port}/open/${encodeURIComponent('o/r#4')}`, { redirect: 'manual' }); + expect(res.status).toBe(302); + expect(logs.some(l => l.includes('did not import') && l.includes('head moved'))).toBe(true); + } finally { + server.close(); + store.close(); + } + }); +}); diff --git a/packages/cli/tests/inbox-prepare.test.ts b/packages/cli/tests/inbox-prepare.test.ts index 70be20a5..f5ad7eff 100644 --- a/packages/cli/tests/inbox-prepare.test.ts +++ b/packages/cli/tests/inbox-prepare.test.ts @@ -137,7 +137,8 @@ describe('the inbox JSON server', () => { const store = new InboxStore(':memory:'); store.observe({ ...snapshot(), headSha: 'aaa' }, true, 'now'); store.markPrepared('o/demo#4', { headSha: 'aaa', bundlePath: '/b.json', worktreePath: '/wt', logPath: '/l', at: 'now' }); - const server = startInboxServer(store, { ...config(), port: 0 }, () => {}); + const noOpenDeps = { baseRefOf: () => 'x', ensureServer: () => Promise.resolve(1), importBundle: () => {} }; + const server = startInboxServer(store, { ...config(), port: 0 }, () => {}, noOpenDeps); await new Promise(resolve => server.on('listening', resolve)); const { port } = server.address() as { port: number }; diff --git a/packages/git/package.json b/packages/git/package.json index 337b6dbb..1774424e 100644 --- a/packages/git/package.json +++ b/packages/git/package.json @@ -1,6 +1,6 @@ { "name": "@diffity/git", - "version": "0.10.9", + "version": "0.10.10", "private": true, "type": "module", "main": "./dist/index.js", diff --git a/packages/github/package.json b/packages/github/package.json index e9435407..917b2dd9 100644 --- a/packages/github/package.json +++ b/packages/github/package.json @@ -1,6 +1,6 @@ { "name": "@diffity/github", - "version": "0.10.9", + "version": "0.10.10", "private": true, "type": "module", "main": "./dist/index.js", diff --git a/packages/parser/package.json b/packages/parser/package.json index 060c5d7b..acff2675 100644 --- a/packages/parser/package.json +++ b/packages/parser/package.json @@ -1,6 +1,6 @@ { "name": "@diffity/parser", - "version": "0.10.9", + "version": "0.10.10", "private": true, "type": "module", "main": "./dist/index.js", diff --git a/packages/ui/package.json b/packages/ui/package.json index 56e97242..68e72558 100644 --- a/packages/ui/package.json +++ b/packages/ui/package.json @@ -1,6 +1,6 @@ { "name": "@diffity/ui", - "version": "0.10.9", + "version": "0.10.10", "type": "module", "private": true, "scripts": {