mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-05 06:59:42 +02:00
Merge branch 'master' into feat/codex-resume
master and this branch both rewrote the two `_claudeSessionId` resets inside
`start()`, so `src/session.ts` conflicted at both of them.
master's commit ccfda623 puts `restoredConversation` at the head of each
fallback chain. A restored mux attach means the CLI never stopped, so a
`/clear` before the Codeman restart may already have moved it to a
conversation the launch id knows nothing about. The persisted chain's tail is
that conversation, and the CLI's own hook reported it first-hand.
This branch adds `this._codexConfig?.resumeSessionId` to the same two chains,
so a resumed codex session keeps its thread-id alias across every mux reattach
and boot recovery.
Both fixes belong. Each chain now reads restoredConversation, then
_resumeSessionId, then omp's alias, then codex's alias, then the launch id.
The comments from both sides are kept.
test/session-claude-conversation-chain.test.ts pins the shape of those two
assignments by matching the source text, and its pattern named omp's alias as
the last term before `this.id`. Codex's alias now sits between the two, so the
pattern widens to pin the ends of the chain and let the middle grow. A `[^;]`
run cannot cross a statement boundary, so each match is still one assignment.
Checked on the merged tree: typecheck, lint, prettier and the frontend syntax
check all pass, and the CI suite runs 6721 tests green across 349 files.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Michael Grundberg
@@ -0,0 +1,78 @@
|
||||
/**
|
||||
* @fileoverview Server wiring for the reverse-proxy base path (#381): prefixed and
|
||||
* unprefixed forms both route, the shell gets the base injected, root-absolute
|
||||
* redirects are rebased without double-prefixing, and a WebSocket upgrade under the
|
||||
* prefix reaches the terminal route. The pure helpers are covered by
|
||||
* test/base-path.test.ts; this boots a real WebServer in test mode.
|
||||
*/
|
||||
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
|
||||
import { WebServer } from '../src/web/server.js';
|
||||
|
||||
const PORT = 3197;
|
||||
|
||||
describe('reverse-proxy base path: server wiring', () => {
|
||||
let server: WebServer;
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
let app: any;
|
||||
beforeAll(async () => {
|
||||
server = new WebServer(PORT, false, true, '127.0.0.1', undefined, false, '/codeman');
|
||||
await server.start();
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
app = (server as any).app;
|
||||
});
|
||||
afterAll(async () => {
|
||||
await server.stop();
|
||||
});
|
||||
|
||||
it('routes prefixed, unprefixed and /api/v1 forms', async () => {
|
||||
for (const url of ['/codeman/api/status', '/api/status', '/codeman/api/v1/status']) {
|
||||
const r = await app.inject({ method: 'GET', url });
|
||||
expect(r.statusCode, url).toBe(200);
|
||||
expect(JSON.parse(r.body).success).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
it('serves the shell with the base injected at /codeman, /codeman/ and /codeman/session/:id', async () => {
|
||||
for (const url of ['/codeman', '/codeman/', '/codeman/session/abc']) {
|
||||
const r = await app.inject({ method: 'GET', url });
|
||||
expect(r.statusCode, url).toBe(200);
|
||||
expect(r.body).toContain('<base href="/codeman/">');
|
||||
expect(r.body).toContain('window.__CODEMAN_BASE__="/codeman"');
|
||||
}
|
||||
});
|
||||
|
||||
it('serves sw.js under the prefix', async () => {
|
||||
const r = await app.inject({ method: 'GET', url: '/codeman/sw.js' });
|
||||
expect(r.statusCode).toBe(200);
|
||||
expect(r.headers['content-type']).toContain('javascript');
|
||||
});
|
||||
|
||||
it('rebases root-absolute redirects and never double-prefixes', async () => {
|
||||
const qr = await app.inject({ method: 'GET', url: '/codeman/q/abcdef' });
|
||||
expect(qr.statusCode).toBe(302);
|
||||
expect(qr.headers.location).toBe('/codeman/');
|
||||
const wv = await app.inject({ method: 'GET', url: '/codeman/webview/somecap' });
|
||||
expect(wv.statusCode).toBe(302);
|
||||
expect(wv.headers.location).toBe('/codeman/webview/somecap/');
|
||||
});
|
||||
|
||||
it('unknown prefixed API path still gets the 404 envelope', async () => {
|
||||
const r = await app.inject({ method: 'GET', url: '/codeman/api/nope' });
|
||||
expect(r.statusCode).toBe(404);
|
||||
expect(JSON.parse(r.body).success).toBe(false);
|
||||
});
|
||||
|
||||
it('routes a prefixed WebSocket upgrade to the terminal route', async () => {
|
||||
const { WebSocket } = await import('ws');
|
||||
const close = (path: string) =>
|
||||
new Promise<{ code: number; reason: string }>((resolve) => {
|
||||
const ws = new WebSocket(`ws://127.0.0.1:${PORT}${path}`, { headers: { origin: `http://127.0.0.1:${PORT}` } });
|
||||
ws.on('close', (code, reason) => resolve({ code, reason: reason.toString() }));
|
||||
ws.on('error', (e) => resolve({ code: -1, reason: String(e) }));
|
||||
});
|
||||
const prefixed = await close('/codeman/ws/sessions/nosuch/terminal');
|
||||
const bare = await close('/ws/sessions/nosuch/terminal');
|
||||
expect(prefixed).toEqual({ code: 4004, reason: 'Session not found' });
|
||||
expect(prefixed).toEqual(bare);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,119 @@
|
||||
/**
|
||||
* @fileoverview Unit tests for the pure reverse-proxy base-path helpers
|
||||
* (src/config/base-path.ts). These back the server ingress strip (rewriteUrl),
|
||||
* the egress Location rewrite (onSend), and the frontend route builder, so their
|
||||
* correctness is what makes a sub-path mount work end to end.
|
||||
*/
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import {
|
||||
normalizeBasePath,
|
||||
isValidBasePath,
|
||||
assertValidBasePath,
|
||||
joinBasePath,
|
||||
stripBasePath,
|
||||
} from '../src/config/base-path.js';
|
||||
|
||||
describe('normalizeBasePath', () => {
|
||||
it('treats root / and empty as no prefix', () => {
|
||||
expect(normalizeBasePath('/')).toBe('');
|
||||
expect(normalizeBasePath('')).toBe('');
|
||||
expect(normalizeBasePath(undefined)).toBe('');
|
||||
expect(normalizeBasePath(null)).toBe('');
|
||||
expect(normalizeBasePath(' ')).toBe('');
|
||||
});
|
||||
|
||||
it('adds a leading slash and drops trailing slashes', () => {
|
||||
expect(normalizeBasePath('codeman')).toBe('/codeman');
|
||||
expect(normalizeBasePath('/codeman')).toBe('/codeman');
|
||||
expect(normalizeBasePath('/codeman/')).toBe('/codeman');
|
||||
expect(normalizeBasePath('codeman///')).toBe('/codeman');
|
||||
});
|
||||
|
||||
it('collapses duplicate slashes and keeps nested segments', () => {
|
||||
expect(normalizeBasePath('//a//b//')).toBe('/a/b');
|
||||
expect(normalizeBasePath('/tools/codeman')).toBe('/tools/codeman');
|
||||
});
|
||||
});
|
||||
|
||||
describe('isValidBasePath / assertValidBasePath', () => {
|
||||
it('accepts root and well-formed segments', () => {
|
||||
expect(isValidBasePath('')).toBe(true);
|
||||
expect(isValidBasePath('/codeman')).toBe(true);
|
||||
expect(isValidBasePath('/tools/codeman-2')).toBe(true);
|
||||
expect(isValidBasePath('/a_b.c~d')).toBe(true);
|
||||
});
|
||||
|
||||
it('rejects segments with unsafe characters', () => {
|
||||
expect(isValidBasePath('/a b')).toBe(false);
|
||||
expect(isValidBasePath('/a?b')).toBe(false);
|
||||
expect(isValidBasePath('/a#b')).toBe(false);
|
||||
expect(isValidBasePath('/a%2f')).toBe(false);
|
||||
});
|
||||
|
||||
it('assertValidBasePath normalizes valid input and throws on bad', () => {
|
||||
expect(assertValidBasePath('/codeman/')).toBe('/codeman');
|
||||
expect(assertValidBasePath('/')).toBe('');
|
||||
expect(() => assertValidBasePath('/a b')).toThrow(/Invalid --base-url/);
|
||||
expect(() => assertValidBasePath('?x')).toThrow(/Invalid --base-url/);
|
||||
});
|
||||
});
|
||||
|
||||
describe('joinBasePath (frontend/egress route builder)', () => {
|
||||
it('is a no-op at root', () => {
|
||||
expect(joinBasePath('', '/api/x')).toBe('/api/x');
|
||||
expect(joinBasePath('', '/')).toBe('/');
|
||||
});
|
||||
|
||||
it('prefixes root-absolute app paths', () => {
|
||||
expect(joinBasePath('/codeman', '/api/x')).toBe('/codeman/api/x');
|
||||
expect(joinBasePath('/codeman', '/')).toBe('/codeman/');
|
||||
expect(joinBasePath('/codeman', '/ws/sessions/1/terminal')).toBe('/codeman/ws/sessions/1/terminal');
|
||||
});
|
||||
|
||||
it('leaves absolute, protocol-relative, and relative URLs alone', () => {
|
||||
expect(joinBasePath('/codeman', 'https://x/y')).toBe('https://x/y');
|
||||
expect(joinBasePath('/codeman', 'ws://x/y')).toBe('ws://x/y');
|
||||
expect(joinBasePath('/codeman', '//host/y')).toBe('//host/y');
|
||||
expect(joinBasePath('/codeman', 'app.js')).toBe('app.js');
|
||||
expect(joinBasePath('/codeman', '#frag')).toBe('#frag');
|
||||
expect(joinBasePath('/codeman', 'data:image/png;base64,AAAA')).toBe('data:image/png;base64,AAAA');
|
||||
});
|
||||
|
||||
it('is idempotent — never double-prefixes', () => {
|
||||
expect(joinBasePath('/codeman', '/codeman/api/x')).toBe('/codeman/api/x');
|
||||
expect(joinBasePath('/codeman', '/codeman')).toBe('/codeman');
|
||||
expect(joinBasePath('/codeman', '/codeman?y=1')).toBe('/codeman?y=1');
|
||||
});
|
||||
|
||||
it('does not treat a same-named sibling path as already-prefixed', () => {
|
||||
// /codeman-docs must NOT be mistaken for the /codeman mount.
|
||||
expect(joinBasePath('/codeman', '/codeman-docs/x')).toBe('/codeman/codeman-docs/x');
|
||||
});
|
||||
});
|
||||
|
||||
describe('stripBasePath (server ingress)', () => {
|
||||
it('is a no-op at root', () => {
|
||||
expect(stripBasePath('', '/api/x')).toBe('/api/x');
|
||||
});
|
||||
|
||||
it('strips the prefix from proxied requests', () => {
|
||||
expect(stripBasePath('/codeman', '/codeman/api/x')).toBe('/api/x');
|
||||
expect(stripBasePath('/codeman', '/codeman')).toBe('/');
|
||||
expect(stripBasePath('/codeman', '/codeman/')).toBe('/');
|
||||
expect(stripBasePath('/codeman', '/codeman?y=1')).toBe('/?y=1');
|
||||
});
|
||||
|
||||
it('leaves un-prefixed requests unchanged (direct-to-port: hooks, health, docker bridge)', () => {
|
||||
expect(stripBasePath('/codeman', '/api/x')).toBe('/api/x');
|
||||
expect(stripBasePath('/codeman', '/api/hook-event')).toBe('/api/hook-event');
|
||||
// A same-named sibling is not the mount.
|
||||
expect(stripBasePath('/codeman', '/codeman-docs/x')).toBe('/codeman-docs/x');
|
||||
});
|
||||
|
||||
it('round-trips with joinBasePath', () => {
|
||||
const base = '/tools/codeman';
|
||||
for (const p of ['/', '/api/x', '/ws/y', '/session/abc']) {
|
||||
expect(stripBasePath(base, joinBasePath(base, p))).toBe(p);
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,59 @@
|
||||
/**
|
||||
* @fileoverview Static guard for the Add Case modal's submit controls (#368).
|
||||
*
|
||||
* Below 860px the shared set-* surface hides the modal footer, and for eight
|
||||
* releases that footer held the only Create/Clone/Link button, so no case could
|
||||
* be added from a phone and nothing failed. This pins the contract that fixed it:
|
||||
* a header submit button exists after the close button, and the two JS paths
|
||||
* that toggle submit state drive BOTH buttons.
|
||||
*/
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { resolve } from 'node:path';
|
||||
|
||||
const publicDir = resolve(import.meta.dirname, '../src/web/public');
|
||||
const html = readFileSync(resolve(publicDir, 'index.html'), 'utf8');
|
||||
const sessionUi = readFileSync(resolve(publicDir, 'session-ui.js'), 'utf8');
|
||||
const mobileCss = readFileSync(resolve(publicDir, 'mobile.css'), 'utf8');
|
||||
|
||||
function caseModal(): string {
|
||||
const start = html.indexOf('<div class="modal" id="createCaseModal">');
|
||||
expect(start).toBeGreaterThan(-1);
|
||||
const next = html.indexOf('<div class="modal"', start + 1);
|
||||
return html.slice(start, next === -1 ? html.length : next);
|
||||
}
|
||||
|
||||
function methodBody(signature: string): string {
|
||||
const start = sessionUi.indexOf(`\n ${signature} {`);
|
||||
expect(start, `${signature} not found in session-ui.js`).toBeGreaterThan(-1);
|
||||
return sessionUi.slice(start, sessionUi.indexOf('\n },', start));
|
||||
}
|
||||
|
||||
describe('Add Case modal submit controls', () => {
|
||||
it('hides the footer on phones, so the header must carry a submit button', () => {
|
||||
expect(mobileCss).toMatch(
|
||||
/:is\(#appSettingsModal, #sessionOptionsModal, #createCaseModal\) \.set-foot \{\s*display: none;/
|
||||
);
|
||||
const modal = caseModal();
|
||||
const head = modal.slice(0, modal.indexOf('<div class="set-body">'));
|
||||
const closeIdx = head.indexOf('class="modal-close"');
|
||||
const saveIdx = head.indexOf('class="set-head-save" id="caseModalSubmitMobile" onclick="app.submitCaseModal()"');
|
||||
expect(closeIdx).toBeGreaterThan(-1);
|
||||
expect(saveIdx).toBeGreaterThan(-1);
|
||||
// Close stays first in the DOM; row-reverse paints Save to its left.
|
||||
expect(closeIdx).toBeLessThan(saveIdx);
|
||||
expect(modal).toContain('id="caseModalSubmit" onclick="app.submitCaseModal()"');
|
||||
});
|
||||
|
||||
it('drives the footer and header submit buttons together', () => {
|
||||
for (const sig of ['switchCaseModalTab(tabName)', 'async submitCaseModal()']) {
|
||||
const body = methodBody(sig);
|
||||
expect(body, sig).toContain("'caseModalSubmit'");
|
||||
expect(body, sig).toContain("'caseModalSubmitMobile'");
|
||||
}
|
||||
});
|
||||
|
||||
it('dims the header button while a submit is pending, where it is the only one visible', () => {
|
||||
expect(mobileCss).toMatch(/#createCaseModal \.set-head-save\.loading \{\s*opacity: 0\.6;\s*pointer-events: none;/);
|
||||
});
|
||||
});
|
||||
@@ -52,6 +52,16 @@ describe('buildWebArgs', () => {
|
||||
]);
|
||||
});
|
||||
|
||||
it('forwards --base-url so a detached/service relaunch keeps the mount prefix', () => {
|
||||
const args = buildWebArgs({ host: '127.0.0.1', port: 3000, https: false, basePath: '/codeman' });
|
||||
expect(args).toContain('--base-url');
|
||||
expect(args[args.indexOf('--base-url') + 1]).toBe('/codeman');
|
||||
});
|
||||
|
||||
it('omits --base-url at root (empty basePath)', () => {
|
||||
expect(buildWebArgs({ host: '127.0.0.1', port: 3000, https: false, basePath: '' })).not.toContain('--base-url');
|
||||
});
|
||||
|
||||
it('never re-emits the daemon flags themselves (the child must not re-fork)', () => {
|
||||
const args = buildWebArgs({ host: '127.0.0.1', port: 3000, https: false });
|
||||
expect(args).not.toContain('--daemon');
|
||||
|
||||
@@ -0,0 +1,453 @@
|
||||
/**
|
||||
* @fileoverview Adopting an ALREADY-RUNNING container (`DockerCase.owned === false`).
|
||||
*
|
||||
* The whole point of adoption is a negative guarantee: Codeman execs into a
|
||||
* container the user built and runs, and never creates, starts, stops, restarts
|
||||
* or removes it. A negative guarantee cannot be observed by using the feature —
|
||||
* only by asserting that the mutating verbs are absent — so these tests read the
|
||||
* generated command strings and assert on what is NOT in them.
|
||||
*
|
||||
* Mirror of the `owned:false` remote-SSH contract (COD-105).
|
||||
*/
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { readFileSync } from 'node:fs';
|
||||
import {
|
||||
defaultDockerCommandForMode,
|
||||
toSessionDocker,
|
||||
isAdoptedContainer,
|
||||
removeDockerContainer,
|
||||
checkDockerConfigDrift,
|
||||
dockerConfigHash,
|
||||
dockerAdoptProbeModes,
|
||||
} from '../src/docker-hosts.js';
|
||||
import { enabledCliIds, getCli } from '../src/config/cli-registry/index.js';
|
||||
import {
|
||||
buildDockerLaunchCommand,
|
||||
buildDockerStopCommand,
|
||||
buildDockerRemoveCommand,
|
||||
buildDockerKillCommand,
|
||||
} from '../src/tmux-manager.js';
|
||||
import type { DockerCase, DockerHost, SessionDocker } from '../src/types.js';
|
||||
|
||||
const HOST: DockerHost = { id: 'h1', label: 'local', engine: 'docker', image: 'codeman/agent:base' };
|
||||
|
||||
function caseFor(owned: boolean | undefined): DockerCase {
|
||||
return {
|
||||
name: 'adopted',
|
||||
type: 'docker',
|
||||
hostId: 'h1',
|
||||
hostWorkspacePath: '/srv/work',
|
||||
container: 'my-own-container',
|
||||
...(owned === undefined ? {} : { owned }),
|
||||
};
|
||||
}
|
||||
|
||||
function launchFor(docker: SessionDocker): string {
|
||||
return buildDockerLaunchCommand({
|
||||
mode: 'codex',
|
||||
docker,
|
||||
sessionId: '11111111-2222-3333-4444-555555555555',
|
||||
createContext: {
|
||||
docker,
|
||||
sessionId: '11111111-2222-3333-4444-555555555555',
|
||||
instance: 'default',
|
||||
userArgs: ['--user', '1000:0'],
|
||||
credentialMounts: [],
|
||||
extraMounts: [],
|
||||
envCreate: { HOME: '/home/agent' },
|
||||
addHostGateway: true,
|
||||
gatewayAlias: 'host.docker.internal',
|
||||
},
|
||||
execEnv: { TERM: 'xterm-256color' },
|
||||
execEnvNames: [],
|
||||
seedCopies: [{ from: '/seed/creds.json', to: '/home/agent/.claude/.credentials.json' }],
|
||||
});
|
||||
}
|
||||
|
||||
describe('adopted container: ownership plumbing', () => {
|
||||
it('carries owned:false from the case onto the live session metadata', () => {
|
||||
expect(toSessionDocker(HOST, caseFor(false)).owned).toBe(false);
|
||||
expect(isAdoptedContainer(toSessionDocker(HOST, caseFor(false)))).toBe(true);
|
||||
});
|
||||
|
||||
it('treats an absent flag as owned, so existing cases are unchanged', () => {
|
||||
const docker = toSessionDocker(HOST, caseFor(undefined));
|
||||
expect(docker.owned).toBeUndefined();
|
||||
expect(isAdoptedContainer(docker)).toBe(false);
|
||||
});
|
||||
|
||||
it('keeps ownership OUT of the config hash so adoption cannot mass-trip drift', () => {
|
||||
// A drift-hash that moved with `owned` would flag every pre-existing case the
|
||||
// moment this field shipped, and the remedy the UI offers is "recreate".
|
||||
const owned = toSessionDocker(HOST, caseFor(undefined));
|
||||
const adopted = toSessionDocker(HOST, caseFor(false));
|
||||
expect(adopted.configHash).toBe(owned.configHash);
|
||||
expect(dockerConfigHash({ ...owned, owned: false } as never)).toBe(owned.configHash);
|
||||
});
|
||||
});
|
||||
|
||||
describe('adopted container: the launch chain never mutates lifecycle', () => {
|
||||
const adopted = launchFor(toSessionDocker(HOST, caseFor(false)));
|
||||
const owned = launchFor(toSessionDocker(HOST, caseFor(undefined)));
|
||||
|
||||
it('never creates the container', () => {
|
||||
expect(owned).toContain('docker create');
|
||||
expect(adopted).not.toContain('docker create');
|
||||
});
|
||||
|
||||
it('never starts the container', () => {
|
||||
expect(owned).toContain('docker start');
|
||||
expect(adopted).not.toContain('docker start');
|
||||
});
|
||||
|
||||
it('never stops or removes the container', () => {
|
||||
for (const verb of ['docker stop', 'docker rm', 'docker restart', 'docker kill']) {
|
||||
expect(adopted).not.toContain(verb);
|
||||
}
|
||||
});
|
||||
|
||||
it('fails closed when the container is missing instead of creating it', () => {
|
||||
expect(adopted).toContain('docker inspect');
|
||||
expect(adopted).toMatch(/not found.*start it yourself/i);
|
||||
});
|
||||
|
||||
it('fails closed when the container is stopped instead of starting it', () => {
|
||||
expect(adopted).toMatch(/\{\{\.State\.Running\}\}/);
|
||||
expect(adopted).toMatch(/not running.*never starts a container it does not own/i);
|
||||
});
|
||||
|
||||
it('uses no double quote and no command substitution in the launch chain', () => {
|
||||
// The whole chain is embedded in an outer `bash -c "…"`. An unescaped `"`
|
||||
// closes that string early, the remainder is re-tokenized, and tmux fails to
|
||||
// exec with a bare `execvp(3) failed: No such file or directory` — no hint
|
||||
// that the command was ever malformed. `$(…)` is banned with it because it
|
||||
// is then evaluated by the wrong shell at the wrong time.
|
||||
expect(adopted).not.toContain('"');
|
||||
expect(adopted).not.toContain('$(');
|
||||
// Every other line already quotes with the single-quote helper.
|
||||
expect(adopted).toContain('grep -qx true');
|
||||
});
|
||||
|
||||
it('skips the base-image gate, which describes an image adoption never uses', () => {
|
||||
expect(owned).toContain('image inspect');
|
||||
expect(adopted).not.toContain('image inspect');
|
||||
});
|
||||
|
||||
it('never seeds host credentials into a container it does not own', () => {
|
||||
expect(owned).toContain('.credentials.json');
|
||||
expect(adopted).not.toContain('.credentials.json');
|
||||
});
|
||||
|
||||
it('still execs into the in-container tmux, which is the whole point', () => {
|
||||
expect(adopted).toContain('docker exec -it');
|
||||
expect(adopted).toContain('new-session -A');
|
||||
});
|
||||
});
|
||||
|
||||
describe('adopted container: the probe request must reach the server', () => {
|
||||
const ui = readFileSync(new URL('../src/web/public/session-ui.js', import.meta.url), 'utf8');
|
||||
const api = readFileSync(new URL('../src/web/public/api-client.js', import.meta.url), 'utf8');
|
||||
|
||||
it('never hands _apiJson an already-stringified body', () => {
|
||||
// _api serializes `body` and sets Content-Type itself. Passing a string
|
||||
// double-encodes it, the server sees a JSON string where it expects an
|
||||
// object, and answers 400 INVALID_INPUT — which the caller reads as "the
|
||||
// container could not be probed", so the menu silently showed every mode.
|
||||
expect(api).toContain('fetchOpts.body = JSON.stringify(body)');
|
||||
const calls = [...ui.matchAll(/_apiJson\([^)]*\{[\s\S]{0,400}?\}\s*\)/g)].map((m) => m[0]);
|
||||
expect(calls.length).toBeGreaterThan(0);
|
||||
for (const call of calls) expect(call).not.toContain('body: JSON.stringify');
|
||||
});
|
||||
|
||||
it('hides every agent mode and says why when the container cannot be read', () => {
|
||||
// Offering claude on a container that is not running is a click that can
|
||||
// only fail, with the reason visible nowhere.
|
||||
// Brace-matched, not a character window: slicing between two call sites
|
||||
// silently yields '' when the second one appears ABOVE the first, and the
|
||||
// assertion then passes over nothing. That has bitten this file twice.
|
||||
const start = ui.indexOf('async _probeDockerCaseModes(activeCase, menu) {');
|
||||
expect(start).toBeGreaterThan(-1);
|
||||
const open = ui.indexOf('{', start);
|
||||
let depth = 0;
|
||||
let fn = '';
|
||||
for (let i = open; i < ui.length; i++) {
|
||||
if (ui[i] === '{') depth++;
|
||||
else if (ui[i] === '}' && --depth === 0) {
|
||||
fn = ui.slice(start, i + 1);
|
||||
break;
|
||||
}
|
||||
}
|
||||
expect(fn).toContain('_dockerCaseProbeError');
|
||||
expect(ui).toContain('_renderRunModeNotice');
|
||||
});
|
||||
});
|
||||
|
||||
describe('adopted container: claude as root', () => {
|
||||
it('drops --dangerously-skip-permissions when the container runs as root', () => {
|
||||
// Claude Code refuses the flag as root ("cannot be used with root/sudo
|
||||
// privileges"), so keeping it kills the pane with a message only visible
|
||||
// inside the container. Our base image runs a non-root user, which is why an
|
||||
// owned container never hit this.
|
||||
expect(defaultDockerCommandForMode('claude', true)).toBe('exec claude');
|
||||
expect(defaultDockerCommandForMode('claude', false)).toContain('--dangerously-skip-permissions');
|
||||
expect(defaultDockerCommandForMode('claude')).toContain('--dangerously-skip-permissions');
|
||||
});
|
||||
|
||||
it('leaves every other mode unchanged as root', () => {
|
||||
for (const mode of ['codex', 'shell', 'pi'] as const) {
|
||||
expect(defaultDockerCommandForMode(mode, true)).toBe(defaultDockerCommandForMode(mode, false));
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('adopted container: the host is not required to have the CLI', () => {
|
||||
const src = readFileSync(new URL('../src/tmux-manager.ts', import.meta.url), 'utf8');
|
||||
|
||||
it('skips the host CLI requirement for a docker session', () => {
|
||||
// A docker session runs its CLI inside the container. Demanding it on the
|
||||
// host threw, the catch fell back to a direct PTY, and that PTY tried to
|
||||
// exec the CLI on the HOST — surfacing as a bare `execvp(3) failed` with
|
||||
// nothing naming the real cause.
|
||||
//
|
||||
// The CLI registry collapsed the old per-mode `mode === 'claude' && !cliDir` chain
|
||||
// into ONE `missingCliMessage(mode)` gate, so the guarantee is now that the single
|
||||
// gate carries the docker exemption and that no per-mode arm has grown back.
|
||||
expect(src).toContain('if (!cliRunsInContainer && !cliDir) {');
|
||||
expect(src.match(/if \(mode === '[a-z]+' && !cliDir\)/g)).toBeNull();
|
||||
});
|
||||
|
||||
it('derives the flag from the docker metadata the session already carries', () => {
|
||||
expect(src).toContain('const cliRunsInContainer = !!docker;');
|
||||
});
|
||||
});
|
||||
|
||||
describe('adopted container: mutating verbs fail closed at the builder', () => {
|
||||
const docker = toSessionDocker(HOST, caseFor(false));
|
||||
|
||||
it('refuses to build a stop command', () => {
|
||||
expect(() => buildDockerStopCommand(docker)).toThrow(/does not own its lifecycle/);
|
||||
});
|
||||
|
||||
it('refuses to build a remove command', () => {
|
||||
expect(() => buildDockerRemoveCommand(docker)).toThrow(/does not own its lifecycle/);
|
||||
});
|
||||
|
||||
it('refuses to remove the container', async () => {
|
||||
await expect(removeDockerContainer(docker)).rejects.toThrow(/does not own its lifecycle/);
|
||||
});
|
||||
|
||||
it('still allows killing THIS session in-container tmux, never the container', () => {
|
||||
const kill = buildDockerKillCommand({ docker, sessionId: 'abcdef12-0000-0000-0000-000000000000' });
|
||||
expect(kill).toContain('tmux');
|
||||
expect(kill).toContain('kill-session');
|
||||
expect(kill).not.toContain('docker stop');
|
||||
expect(kill).not.toContain('docker rm');
|
||||
});
|
||||
|
||||
it('still permits every verb for an owned container', () => {
|
||||
const ownedDocker = toSessionDocker(HOST, caseFor(undefined));
|
||||
expect(buildDockerStopCommand(ownedDocker)).toContain('stop -t 10');
|
||||
expect(buildDockerRemoveCommand(ownedDocker)).toContain('rm -f');
|
||||
});
|
||||
});
|
||||
|
||||
describe('adopted container: the Add Case panel id contract', () => {
|
||||
// The modal's load/save contract is getElementById by fixed id, so a renamed or
|
||||
// dropped id stops the control working with no error anywhere. Static guard in
|
||||
// the style of app-settings-structure / session-options-structure.
|
||||
const html = readFileSync(new URL('../src/web/public/index.html', import.meta.url), 'utf8');
|
||||
const ui = readFileSync(new URL('../src/web/public/session-ui.js', import.meta.url), 'utf8');
|
||||
const css = readFileSync(new URL('../src/web/public/styles.css', import.meta.url), 'utf8');
|
||||
|
||||
it('ships every id session-ui.js reads back', () => {
|
||||
for (const id of ['dockerAdoptExisting', 'dockerContainerName', 'dockerAdoptCheckBtn']) {
|
||||
expect(html).toContain(`id="${id}"`);
|
||||
expect(ui).toContain(`'${id}'`);
|
||||
}
|
||||
});
|
||||
|
||||
it('routes adoption to the endpoint that never creates a container', () => {
|
||||
expect(ui).toContain('/api/cases/docker-adopt');
|
||||
expect(ui).toContain('/api/docker-cases/adopt-preflight');
|
||||
// The create path must survive untouched beside it.
|
||||
expect(ui).toContain('/api/cases/docker-link');
|
||||
});
|
||||
|
||||
it('hides the adopt-only row until the toggle is on, so the panel is unchanged by default', () => {
|
||||
expect(css).toContain('#createCaseModal .docker-adopt-only');
|
||||
expect(css).toMatch(/#createCaseModal \.docker-adopt-only \{\s*display: none/);
|
||||
expect(css).toContain("#createCaseModal[data-docker-adopt='1'] .docker-adopt-only");
|
||||
});
|
||||
|
||||
it('marks the create-time rows so adoption hides the fields it never uses', () => {
|
||||
// image / network / advanced describe a `docker create` adoption never runs.
|
||||
expect(html.match(/docker-create-only/g)?.length).toBeGreaterThanOrEqual(3);
|
||||
expect(css).toContain("#createCaseModal[data-docker-adopt='1'] .docker-create-only");
|
||||
});
|
||||
});
|
||||
|
||||
describe('adopted container: run modes come from the CONTAINER, not the host', () => {
|
||||
const ui = readFileSync(new URL('../src/web/public/session-ui.js', import.meta.url), 'utf8');
|
||||
/** Slice the method BODY. Anchored on the definition, not a call site: the
|
||||
* menu opener calls _loadRunModeHistory() ABOVE this definition, so slicing
|
||||
* between call sites silently yields an empty string and passes nothing. */
|
||||
/**
|
||||
* The method BODY, delimited by brace depth rather than a character budget.
|
||||
* A fixed window silently truncates the moment the method grows — which is
|
||||
* exactly what happened twice: a comment added above the assertion pushed the
|
||||
* asserted line past the cutoff and CI failed on a test that was still true.
|
||||
*/
|
||||
const refreshFn = (src) => {
|
||||
const start = src.indexOf('_refreshRunModeAvailability(menu) {');
|
||||
expect(start).toBeGreaterThan(-1);
|
||||
const open = src.indexOf('{', start);
|
||||
let depth = 0;
|
||||
for (let i = open; i < src.length; i++) {
|
||||
if (src[i] === '{') depth++;
|
||||
else if (src[i] === '}' && --depth === 0) return src.slice(start, i + 1);
|
||||
}
|
||||
throw new Error('unbalanced braces in _refreshRunModeAvailability');
|
||||
};
|
||||
|
||||
it('gates a docker case on availableModes instead of host CLI probes', () => {
|
||||
// The sandbox host had codex but no claude while the adopted container had
|
||||
// claude and no codex; gating on the host hid the only mode that worked.
|
||||
const fn = refreshFn(ui);
|
||||
expect(fn).toContain("location === 'docker'");
|
||||
expect(fn).toContain('availableModes');
|
||||
// Non-docker cases must keep the original host probe (#201).
|
||||
expect(fn).toContain('this.isCliAvailable(mode)');
|
||||
});
|
||||
|
||||
it('leaves an owned container ungated when nothing was probed', () => {
|
||||
// Our base image ships every CLI, so an absent list means "unknown", and
|
||||
// treating unknown as "nothing available" would empty the menu.
|
||||
expect(refreshFn(ui)).toMatch(/containerModes \?[^:]*:\s*true/);
|
||||
});
|
||||
});
|
||||
|
||||
describe('adopted container: both path fields get a folder picker', () => {
|
||||
const html = readFileSync(new URL('../src/web/public/index.html', import.meta.url), 'utf8');
|
||||
const ui = readFileSync(new URL('../src/web/public/session-ui.js', import.meta.url), 'utf8');
|
||||
const picker = readFileSync(new URL('../src/web/public/keyboard-accessory.js', import.meta.url), 'utf8');
|
||||
|
||||
it('wires a Browse button to each of the two paths', () => {
|
||||
expect(html).toContain('app.openDockerWorkspacePathPicker()');
|
||||
expect(html).toContain('app.openDockerWorkdirPicker()');
|
||||
// Same markup Link Existing uses, so the two look and behave alike.
|
||||
expect(html.match(/path-input-browse/g)?.length).toBeGreaterThanOrEqual(3);
|
||||
});
|
||||
|
||||
it('browses the CONTAINER for the container workdir, not the host', () => {
|
||||
// For an adopted container nothing is mounted at a matching host path, so a
|
||||
// host listing would be a different filesystem — and typing this field blind
|
||||
// is what makes the launch fail with an OCI chdir error.
|
||||
const fn = ui.slice(ui.indexOf('openDockerWorkdirPicker()'), ui.indexOf('async linkRemoteCase()'));
|
||||
expect(fn).toContain('/api/docker-cases/browse');
|
||||
expect(fn).not.toContain('/api/filesystem/browse');
|
||||
expect(fn).toContain('fetchListing');
|
||||
});
|
||||
|
||||
it('keeps the host picker for the host workspace path', () => {
|
||||
const fn = ui.slice(ui.indexOf('openDockerWorkspacePathPicker()'), ui.indexOf('openDockerWorkdirPicker()'));
|
||||
expect(fn).toContain('PathPicker.open');
|
||||
expect(fn).not.toContain('fetchListing');
|
||||
});
|
||||
|
||||
it('reuses one PathPicker via an optional source rather than forking it', () => {
|
||||
expect(picker).toContain('this._options.fetchListing');
|
||||
expect(picker).toContain('/api/filesystem/browse');
|
||||
});
|
||||
});
|
||||
|
||||
describe('adopted container: drift is not evaluated', () => {
|
||||
it('reports no drift rather than demanding a recreate we may not perform', async () => {
|
||||
// An adopted container carries no codeman.confighash label, so a real
|
||||
// comparison would always report drift and the launch gate would 409 forever.
|
||||
const status = await checkDockerConfigDrift(toSessionDocker(HOST, caseFor(false)));
|
||||
expect(status.drifted).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('adopted container: probe modes come from the CLI registry', () => {
|
||||
it('probes every enabled CLI, so a newly-enabled one needs no second list', () => {
|
||||
// A hand-written list here silently froze: `omp` shipped in 1.24.0 and was
|
||||
// missing from it, which hid the omp run mode on EVERY docker case — owned
|
||||
// ones included, since the run menu gates on this same probe.
|
||||
const modes = dockerAdoptProbeModes();
|
||||
expect(modes).toEqual(enabledCliIds());
|
||||
expect(modes).toContain('omp');
|
||||
expect(modes).toContain('shell');
|
||||
});
|
||||
|
||||
it('resolves the real binary name, not the mode name', () => {
|
||||
// `antigravity` ships as `agy` and `deepseek` as `dsh`, so a mode-name probe
|
||||
// would report both as missing on a container that has them.
|
||||
expect(getCli('antigravity')?.discovery.binaries[0]).toBe('agy');
|
||||
expect(getCli('deepseek')?.discovery.binaries[0]).toBe('dsh');
|
||||
expect(getCli('shell')?.discovery.binaries[0]).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('adopted container: export never touches the container', () => {
|
||||
const routes = readFileSync(new URL('../src/web/routes/case-routes.ts', import.meta.url), 'utf8');
|
||||
const exporter = readFileSync(new URL('../src/docker-export.ts', import.meta.url), 'utf8');
|
||||
|
||||
it('refuses a full-image export, which would commit a container we do not own', () => {
|
||||
expect(routes).toContain("if (mode === 'full' && dockerCase.owned === false)");
|
||||
});
|
||||
|
||||
it('never pauses an adopted container for the workspace tar', () => {
|
||||
// `docker pause` freezes the owner's processes for as long as the tar takes.
|
||||
// It is the one export step that touches the container at all.
|
||||
expect(exporter).toContain('!isAdoptedContainer(docker) && (await isContainerRunning(');
|
||||
});
|
||||
});
|
||||
|
||||
describe('adopted container: naming a foreign container is machine-level', () => {
|
||||
const routes = readFileSync(new URL('../src/web/routes/case-routes.ts', import.meta.url), 'utf8');
|
||||
const routeFor = (marker: string) => routes.slice(routes.indexOf(marker), routes.indexOf(marker) + 1400);
|
||||
|
||||
it('admin-gates adoption in multi-user mode, unlike docker-link', () => {
|
||||
// docker-link only ever creates OUR container, whose sole bind mount is a
|
||||
// workspace isWorkingDirAllowed already confined. An adopted container's
|
||||
// mounts belong to its owner — one mounting `/` hands the adopter the host.
|
||||
expect(routeFor("'/api/cases/docker-adopt'")).toContain('adminOnly(req, reply)');
|
||||
});
|
||||
|
||||
it('admin-gates enumerating and browsing containers', () => {
|
||||
expect(routeFor("'/api/docker-hosts/:hostId/containers'")).toContain('adminOnly(req, reply)');
|
||||
expect(routeFor("'/api/docker-cases/browse'")).toContain('adminOnly(req, reply)');
|
||||
});
|
||||
|
||||
it('lets a non-admin preflight only a container linked to a case they own', () => {
|
||||
// NOT plain adminOnly: the run menu probes this for every docker case to learn
|
||||
// which CLIs the container has, so an admin-only gate would hide every agent
|
||||
// mode from a non-admin's own docker case.
|
||||
const route = routeFor("'/api/docker-cases/adopt-preflight'");
|
||||
expect(route).toContain('if (!isAdmin(req))');
|
||||
expect(route).toContain('canAccessOwned(getAuthUser(req), item.owner)');
|
||||
expect(route).not.toContain('adminOnly(req, reply)');
|
||||
});
|
||||
});
|
||||
|
||||
describe('adopted container: a missing container means different things per ownership', () => {
|
||||
const ui = readFileSync(new URL('../src/web/public/session-ui.js', import.meta.url), 'utf8');
|
||||
const routes = readFileSync(new URL('../src/web/routes/case-routes.ts', import.meta.url), 'utf8');
|
||||
|
||||
it('records a probe failure only for an adopted case', () => {
|
||||
// An OWNED container does not exist until the first session launches it, so
|
||||
// "not found" is the expected answer for every freshly linked Docker case.
|
||||
// Treating it as a fault hid every agent mode behind an error telling the user
|
||||
// to start a container the launch chain was about to create itself.
|
||||
const probe = ui.slice(ui.indexOf('async _probeDockerCaseModes('), ui.indexOf('async _loadRunModeHistory('));
|
||||
expect(probe).toContain('if (activeCase?.docker?.owned === false) {');
|
||||
expect(probe.indexOf('if (activeCase?.docker?.owned === false) {')).toBeLessThan(
|
||||
probe.indexOf('this._dockerCaseProbeError[name] =')
|
||||
);
|
||||
});
|
||||
|
||||
it('ships the ownership flag the UI reads that decision from', () => {
|
||||
expect(routes).toContain('...(dockerCase.owned === false ? { owned: false } : {}),');
|
||||
});
|
||||
});
|
||||
@@ -161,6 +161,8 @@ function loadPanel(options: { sessionId?: string | null; showHidden?: boolean }
|
||||
CodemanApp,
|
||||
console,
|
||||
escapeHtml,
|
||||
// Reverse-proxy route builder from constants.js (not loaded here); identity at root.
|
||||
CodemanBase: { base: '', url: (p: string) => p },
|
||||
localStorage: { getItem: () => null, setItem: vi.fn() },
|
||||
document: {
|
||||
getElementById: (id: string) => elements[id] ?? null,
|
||||
@@ -222,6 +224,8 @@ function loadRealSelectSessionHarness(options: { terminalFailure?: boolean } = {
|
||||
},
|
||||
HTMLCanvasElement: class HTMLCanvasElement {},
|
||||
WebSocket: { OPEN: 1 },
|
||||
// Reverse-proxy route builder from constants.js (not loaded here); identity at root.
|
||||
CodemanBase: { base: '', url: (p: string) => p },
|
||||
MobileDetection: { isTouchDevice: () => false },
|
||||
localStorage: { length: 0, key: vi.fn(), getItem: vi.fn(), setItem: vi.fn(), removeItem: vi.fn() },
|
||||
document: {
|
||||
|
||||
@@ -0,0 +1,53 @@
|
||||
/**
|
||||
* @fileoverview The picker must offer a root when Codeman runs as root.
|
||||
*
|
||||
* `/root` is a DEFAULT blocked tree in the attachment guard, and Codeman running
|
||||
* as root — containers, plenty of servers — makes `homedir()` exactly `/root`.
|
||||
* The picker's own allowlisted Home root was therefore blocked by the guard,
|
||||
* every other candidate lives under it or does not exist, and the endpoint
|
||||
* answered 403 "No filesystem browse roots are available" with nothing the user
|
||||
* could open. The fix drops only the trees that would swallow a configured root
|
||||
* whole; `isSensitivePath` still guards what is inside.
|
||||
*/
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { isBlockedAttachmentPath, isUnderTree } from '../src/config/attachment-guard.js';
|
||||
|
||||
const TREES = ['/root', '/etc'];
|
||||
|
||||
/** Mirror of pickerBlockedTrees in file-routes.ts. */
|
||||
const narrow = (trees: readonly string[], roots: readonly string[]) =>
|
||||
roots.length === 0 ? trees : trees.filter((t) => !roots.some((r) => isUnderTree(r, t)));
|
||||
|
||||
describe('file picker roots when the server runs as root', () => {
|
||||
it('drops the tree that would swallow the configured Home root', () => {
|
||||
expect(narrow(TREES, ['/root'])).toEqual(['/etc']);
|
||||
});
|
||||
|
||||
it('keeps trees that hold no configured root', () => {
|
||||
expect(narrow(TREES, ['/home/alice'])).toEqual(['/root', '/etc']);
|
||||
expect(narrow(TREES, [])).toEqual(['/root', '/etc']);
|
||||
});
|
||||
|
||||
it('also frees a root nested under the blocked tree', () => {
|
||||
// ~/codeman-cases is /root/codeman-cases when running as root.
|
||||
expect(narrow(TREES, ['/root/codeman-cases'])).toEqual(['/etc']);
|
||||
});
|
||||
|
||||
it('still refuses secrets inside the freed tree', () => {
|
||||
const trees = narrow(TREES, ['/root']);
|
||||
for (const p of ['/root/.ssh/id_rsa', '/root/.aws/credentials', '/root/app/.env']) {
|
||||
expect(isBlockedAttachmentPath(p, trees)).toBe(true);
|
||||
}
|
||||
// …while ordinary files under it become reachable, which is the point.
|
||||
expect(isBlockedAttachmentPath('/root/projects/readme.md', trees)).toBe(false);
|
||||
});
|
||||
|
||||
it('navigation reuses the same narrowed list the roots were chosen with', () => {
|
||||
// Handing the raw trees to navigation would admit a root and then refuse
|
||||
// every path inside it — a picker that opens and then does nothing.
|
||||
const src = readFileSync(new URL('../src/web/routes/file-routes.ts', import.meta.url), 'utf8');
|
||||
expect(src.match(/pickerBlockedTrees\(/g)?.length).toBeGreaterThanOrEqual(3);
|
||||
expect(src).not.toMatch(/blockedTrees:\s*guard\.blockedTrees/);
|
||||
});
|
||||
});
|
||||
@@ -38,6 +38,8 @@ function loadApp() {
|
||||
console: { ...console, warn: vi.fn(), error: vi.fn() },
|
||||
localStorage: { getItem: () => null, setItem: () => {}, removeItem: () => {} },
|
||||
escapeHtml: (s: string) => String(s),
|
||||
// Reverse-proxy route builder from constants.js (not loaded here); identity at root.
|
||||
CodemanBase: { base: '', url: (p: string) => p },
|
||||
document: { getElementById: () => null, addEventListener: vi.fn() },
|
||||
window: windowStub,
|
||||
setTimeout,
|
||||
|
||||
@@ -64,6 +64,8 @@ function loadApp(media: FakeMedia[]) {
|
||||
console: { ...console, warn: vi.fn() },
|
||||
localStorage: { getItem: () => null, setItem: () => {}, removeItem: () => {} },
|
||||
escapeHtml: (s: string) => String(s),
|
||||
// Reverse-proxy route builder from constants.js (not loaded here); identity at root.
|
||||
CodemanBase: { base: '', url: (p: string) => p },
|
||||
document: { getElementById: () => null, addEventListener: vi.fn() },
|
||||
window: { addEventListener: vi.fn() },
|
||||
setTimeout,
|
||||
|
||||
@@ -24,7 +24,15 @@ describe('frontend public asset tooling', () => {
|
||||
const appJs = readFileSync(resolve(repoRoot, 'src/web/public/app.js'), 'utf8');
|
||||
|
||||
expect(appJs).toContain("body.appendChild(this._buildResponseViewerMessage(lastResponse, 'assistant'");
|
||||
expect(appJs).toContain('body.appendChild(this._buildResponseViewerMessage(msg.text, msg.role, agentLabel));');
|
||||
expect(appJs).toContain(
|
||||
'body.appendChild(this._buildResponseViewerMessage(msg.text, msg.role, agentLabel, { ...msg, continuation }));'
|
||||
);
|
||||
// ⚠️ A numeric `turn` gates continuation rendering. Only the Claude reader
|
||||
// emits turns; Codex, the external-CLI pane parser and an older server emit
|
||||
// adjacent same-role messages with none, and must keep one badge per card.
|
||||
expect(appJs).toContain(
|
||||
"!!previous && previous.role === msg.role && typeof msg.turn === 'number' && previous.turn === msg.turn"
|
||||
);
|
||||
expect(appJs).toContain("div.className = 'rv-message ' + (isUser ? 'rv-msg-user' : 'rv-msg-assistant');");
|
||||
expect(appJs).toContain("renderedText.className = 'rv-text';");
|
||||
});
|
||||
|
||||
@@ -42,6 +42,34 @@ describe('generateHooksConfig', () => {
|
||||
expect(config.hooks.Stop).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('reports the live conversation id on every prompt, with stdout discarded', () => {
|
||||
const config = generateHooksConfig();
|
||||
expect(config.hooks.UserPromptSubmit).toBeInstanceOf(Array);
|
||||
expect(config.hooks.UserPromptSubmit).toHaveLength(1);
|
||||
const command = (config.hooks.UserPromptSubmit as Array<{ hooks: Array<{ command: string }> }>)[0].hooks[0].command;
|
||||
expect(command).toContain('"event":"prompt_submitted"');
|
||||
expect(command).toContain('$CODEMAN_SESSION_ID');
|
||||
// ⚠️ Claude Code injects a UserPromptSubmit hook's stdout into the model's
|
||||
// context ("Exit code 0 - stdout shown to Claude"), so without this the API
|
||||
// envelope is pasted into the user's own prompt on every turn. Every other
|
||||
// event's stdout is harmless (it feeds SSE).
|
||||
// ⚠️ Assert curl's OWN flag, not a trailing redirect: the command already
|
||||
// ends `… 2>/dev/null || true`, and in `pipeline || true >/dev/null` the
|
||||
// shell binds the redirect to `true`, which never runs on the success path.
|
||||
// A `endsWith('>/dev/null')` assertion passes on exactly that broken form.
|
||||
expect(command).toContain('curl -sk -o /dev/null -X POST');
|
||||
expect(command.trimEnd().endsWith('>/dev/null')).toBe(false);
|
||||
});
|
||||
|
||||
it("leaves every other hook event's command text byte-identical", () => {
|
||||
// The opt-in discard exists so the five SSE-fed events do not change shape:
|
||||
// rewriting their command churns every workspace's settings.local.json.
|
||||
const config = generateHooksConfig();
|
||||
const stop = (config.hooks.Stop as Array<{ hooks: Array<{ command: string }> }>)[0].hooks[0].command;
|
||||
expect(stop).toContain('curl -sk -X POST');
|
||||
expect(stop).not.toContain('-o /dev/null');
|
||||
});
|
||||
|
||||
it('should guard subagent stops while their background work is active', () => {
|
||||
const config = generateHooksConfig();
|
||||
const subagentHooks = config.hooks.SubagentStop as Array<{
|
||||
@@ -324,6 +352,39 @@ describe('writeHooksConfig', () => {
|
||||
expect(serialized).not.toContain('CODEMAN_BACKGROUND_REWAKE_V1');
|
||||
});
|
||||
|
||||
it('heals a hooks block written before UserPromptSubmit existed', async () => {
|
||||
const claudeDir = join(testDir, '.claude');
|
||||
const settingsPath = join(claudeDir, 'settings.local.json');
|
||||
mkdirSync(claudeDir, { recursive: true });
|
||||
// An otherwise-current block from the previous release: the pane would keep
|
||||
// guessing its conversation from ~/.claude/history.jsonl forever.
|
||||
const hooks = generateHooksConfig().hooks;
|
||||
delete hooks.UserPromptSubmit;
|
||||
writeFileSync(settingsPath, JSON.stringify({ hooks }, null, 2));
|
||||
|
||||
await refreshStaleCodemanHooks(testDir);
|
||||
|
||||
const parsed = JSON.parse(readFileSync(settingsPath, 'utf-8'));
|
||||
expect(JSON.stringify(parsed.hooks.UserPromptSubmit)).toContain('prompt_submitted');
|
||||
});
|
||||
|
||||
it('leaves an already-current hooks block untouched', async () => {
|
||||
// ⚠️ The staleness gate reads a JSON.stringify'd blob, so a marker written
|
||||
// with surrounding quotes never matches and the gate is permanently false —
|
||||
// which rewrites every workspace's settings.local.json on every Claude
|
||||
// spawn instead of never. This asserts the no-op, which is the property a
|
||||
// quoted needle silently breaks.
|
||||
const claudeDir = join(testDir, '.claude');
|
||||
const settingsPath = join(claudeDir, 'settings.local.json');
|
||||
mkdirSync(claudeDir, { recursive: true });
|
||||
writeFileSync(settingsPath, JSON.stringify({ hooks: generateHooksConfig().hooks }, null, 2));
|
||||
const before = readFileSync(settingsPath, 'utf-8');
|
||||
|
||||
await refreshStaleCodemanHooks(testDir);
|
||||
|
||||
expect(readFileSync(settingsPath, 'utf-8')).toBe(before);
|
||||
});
|
||||
|
||||
it('replaces the V2 background hook without duplicating it', async () => {
|
||||
const claudeDir = join(testDir, '.claude');
|
||||
const settingsPath = join(claudeDir, 'settings.local.json');
|
||||
|
||||
@@ -29,6 +29,32 @@ export class MockSession extends EventEmitter {
|
||||
terminalBuffer: string = '';
|
||||
/** Mirrors Session.lastSubmitAt — the response viewer credits history entries by it. */
|
||||
lastSubmitAt: number = 0;
|
||||
/** Mirrors Session.claudeSessionId — the conversation the viewer reads. */
|
||||
claudeSessionId: string | null = null;
|
||||
/** Mirrors Session.claudeSessionIdIsFirstHand — set only by a hook adoption. */
|
||||
claudeSessionIdIsFirstHand: boolean = false;
|
||||
/** Mirrors Session.claudeSessionChain — oldest first, current last. */
|
||||
claudeSessionChain: string[] = [];
|
||||
|
||||
/** Mirrors Session.adoptClaudeSessionId, including the first-hand chain rule. */
|
||||
adoptClaudeSessionId(newId: string, options: { firstHand?: boolean } = {}): void {
|
||||
if (!newId) return;
|
||||
if (options.firstHand) {
|
||||
this.claudeSessionIdIsFirstHand = true;
|
||||
if (this.claudeSessionChain[this.claudeSessionChain.length - 1] !== newId) {
|
||||
const existing = this.claudeSessionChain.indexOf(newId);
|
||||
if (existing !== -1) this.claudeSessionChain.splice(existing, 1);
|
||||
this.claudeSessionChain.push(newId);
|
||||
}
|
||||
}
|
||||
if (newId === this.claudeSessionId) return;
|
||||
this.claudeSessionId = newId;
|
||||
}
|
||||
|
||||
/** Mirrors Session.markPromptSubmitted. */
|
||||
markPromptSubmitted(): void {
|
||||
this.lastSubmitAt = Date.now();
|
||||
}
|
||||
|
||||
private _muxName: string | null = null;
|
||||
|
||||
|
||||
@@ -0,0 +1,249 @@
|
||||
/**
|
||||
* @fileoverview The PR bot's Telegram command and button handling, driven through
|
||||
* `PrBot.handleUpdate` with a recording Telegram stub and a mocked `gh` layer. Pins
|
||||
* the one property that matters most: a GitHub write (merge, close, post) happens only
|
||||
* after the confirmation tap, exactly once, and never for a foreign chat or a stale
|
||||
* nonce.
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||
import { mkdtempSync } from 'fs';
|
||||
import { tmpdir } from 'os';
|
||||
import { join } from 'path';
|
||||
|
||||
const gh = vi.hoisted(() => ({
|
||||
listOpenPrs: vi.fn(async () => []),
|
||||
getPrDetail: vi.fn(),
|
||||
getCiStatus: vi.fn(async () => ({ state: 'passed', runs: [] })),
|
||||
mergePr: vi.fn(async () => 'merged'),
|
||||
closePr: vi.fn(async () => 'closed'),
|
||||
commentPr: vi.fn(async () => 'commented'),
|
||||
approveWorkflowRun: vi.fn(async () => undefined),
|
||||
gh: vi.fn(async () => 'MERGED\n'),
|
||||
}));
|
||||
vi.mock('../scripts/pr-bot/github.js', () => gh);
|
||||
vi.mock('../scripts/pr-bot/worktree.js', () => ({
|
||||
preparePrWorktree: vi.fn(),
|
||||
removePrWorktree: vi.fn(async () => undefined),
|
||||
}));
|
||||
|
||||
import { PrBot, type TelegramLike } from '../scripts/pr-bot/bot.js';
|
||||
import { buildConfig } from '../scripts/pr-bot/config.js';
|
||||
import type { CodemanClient } from '../scripts/pr-bot/codeman-client.js';
|
||||
import type { PrDetail } from '../scripts/pr-bot/github.js';
|
||||
import type { ReviewReport } from '../scripts/pr-bot/report.js';
|
||||
|
||||
class FakeTelegram implements TelegramLike {
|
||||
sent: { text: string; markup?: unknown; plain: boolean }[] = [];
|
||||
edits: number[] = [];
|
||||
private nextId = 100;
|
||||
isOurChat(chatId: number | string | undefined): boolean {
|
||||
return String(chatId) === '1';
|
||||
}
|
||||
async sendMessage(text: string, opts: { replyMarkup?: unknown } = {}): Promise<number> {
|
||||
this.sent.push({ text, markup: opts.replyMarkup, plain: false });
|
||||
return this.nextId++;
|
||||
}
|
||||
async sendPlain(text: string): Promise<number> {
|
||||
this.sent.push({ text, plain: true });
|
||||
return this.nextId++;
|
||||
}
|
||||
async editReplyMarkup(messageId: number): Promise<void> {
|
||||
this.edits.push(messageId);
|
||||
}
|
||||
async deleteMessage(): Promise<void> {}
|
||||
async answerCallback(): Promise<void> {}
|
||||
async sendDocument(): Promise<void> {}
|
||||
async getUpdates(): Promise<[]> {
|
||||
return [];
|
||||
}
|
||||
async setMyCommands(): Promise<void> {}
|
||||
last(): string {
|
||||
return this.sent[this.sent.length - 1]?.text ?? '';
|
||||
}
|
||||
/** The confirm callback_data of the last message's keyboard. */
|
||||
confirmData(): string {
|
||||
const markup = this.sent[this.sent.length - 1]?.markup as { inline_keyboard: { callback_data: string }[][] };
|
||||
return markup.inline_keyboard.flat().find((b) => b.callback_data.startsWith('confirm:'))!.callback_data;
|
||||
}
|
||||
}
|
||||
|
||||
function detail(over: Partial<PrDetail> = {}): PrDetail {
|
||||
return {
|
||||
number: 381,
|
||||
title: 'feat(web): base URL',
|
||||
author: 'mtiller',
|
||||
headSha: 'abc123abc123',
|
||||
baseRef: 'master',
|
||||
headRef: 'feat',
|
||||
isDraft: false,
|
||||
mergeable: 'MERGEABLE',
|
||||
mergeState: 'CLEAN',
|
||||
additions: 10,
|
||||
deletions: 2,
|
||||
changedFiles: 3,
|
||||
updatedAt: '',
|
||||
url: 'https://github.com/Ark0N/Codeman/pull/381',
|
||||
isCrossRepository: true,
|
||||
labels: [],
|
||||
body: '',
|
||||
files: [],
|
||||
authorAssociation: 'CONTRIBUTOR',
|
||||
linkedIssues: [],
|
||||
commitCount: 1,
|
||||
commentCount: 0,
|
||||
reviewDecision: '',
|
||||
headRepo: 'mtiller/Codeman',
|
||||
...over,
|
||||
};
|
||||
}
|
||||
|
||||
const report: ReviewReport = {
|
||||
verdict: 'merge',
|
||||
confidence: 'high',
|
||||
summary: 's',
|
||||
changes: [],
|
||||
findings: [],
|
||||
checks: [],
|
||||
scope: 'focused',
|
||||
risk: '',
|
||||
recommendation: 'merge it',
|
||||
draftComment: 'Thanks, merging.',
|
||||
assumptions: [],
|
||||
};
|
||||
|
||||
const msg = (text: string, chat = 1, replyTo?: { message_id: number; text?: string }) => ({
|
||||
update_id: 1,
|
||||
message: { message_id: 7, chat: { id: chat }, text, reply_to_message: replyTo },
|
||||
});
|
||||
const cb = (data: string, chat = 1) => ({
|
||||
update_id: 2,
|
||||
callback_query: { id: 'q', from: { id: 1 }, data, message: { message_id: 9, chat: { id: chat } } },
|
||||
});
|
||||
|
||||
describe('PrBot commands', () => {
|
||||
let bot: PrBot;
|
||||
let tg: FakeTelegram;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
gh.getPrDetail.mockImplementation(async () => detail());
|
||||
const cfg = buildConfig(
|
||||
{ TELEGRAM_BOT_TOKEN: 't', TELEGRAM_CHAT_ID: '1', PR_BOT_DATA_DIR: mkdtempSync(join(tmpdir(), 'prbot-cmd-')) },
|
||||
{ home: '/h', repoRoot: '/r' }
|
||||
);
|
||||
tg = new FakeTelegram();
|
||||
bot = new PrBot(cfg, { telegram: tg, codeman: {} as CodemanClient, log: () => undefined });
|
||||
const rec = bot.store.upsertPr(detail());
|
||||
Object.assign(rec, { status: 'reviewed', reviewedSha: 'abc123abc123', verdict: 'merge-with-fixes', report });
|
||||
bot.store.save();
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
await bot.stop();
|
||||
});
|
||||
|
||||
it('answers /help only for the configured chat', async () => {
|
||||
await bot.handleUpdate(msg('/help', 2));
|
||||
expect(tg.sent).toHaveLength(0);
|
||||
await bot.handleUpdate(msg('/help'));
|
||||
expect(tg.last()).toContain('/merge N');
|
||||
});
|
||||
|
||||
it('shows the draft without posting it', async () => {
|
||||
await bot.handleUpdate(msg('/draft 381'));
|
||||
expect(tg.last()).toContain('Thanks, merging.');
|
||||
expect(tg.last()).toContain('not posted');
|
||||
expect(gh.commentPr).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('merges only after the confirmation tap, once, and rejects a reused nonce', async () => {
|
||||
await bot.handleUpdate(msg('/merge 381'));
|
||||
expect(gh.mergePr).not.toHaveBeenCalled();
|
||||
expect(tg.last()).toContain('Merge <b>#381</b>');
|
||||
expect(Object.keys(bot.store.state.pending)).toHaveLength(1);
|
||||
const data = tg.confirmData();
|
||||
expect(data).toMatch(/^confirm:merge:381:[0-9a-f]{8}$/);
|
||||
|
||||
await bot.handleUpdate(cb(data));
|
||||
expect(gh.mergePr).toHaveBeenCalledTimes(1);
|
||||
expect(gh.mergePr).toHaveBeenCalledWith('Ark0N/Codeman', 381);
|
||||
expect(tg.last()).toContain('Merged <b>#381</b>');
|
||||
expect(Object.keys(bot.store.state.pending)).toHaveLength(0);
|
||||
expect(tg.edits).toContain(9); // the keyboard is removed from the confirmation message
|
||||
|
||||
await bot.handleUpdate(cb(data));
|
||||
expect(gh.mergePr).toHaveBeenCalledTimes(1);
|
||||
expect(tg.last()).toContain('no longer valid');
|
||||
});
|
||||
|
||||
it('announces a bot-made merge once: the next scan retires the PR silently', async () => {
|
||||
await bot.handleUpdate(msg('/merge 381'));
|
||||
await bot.handleUpdate(cb(tg.confirmData()));
|
||||
const merged = tg.sent.filter((s) => s.text.includes('Merged <b>#381</b>'));
|
||||
expect(merged).toHaveLength(1);
|
||||
expect(merged[0].text).toContain('fixes to apply at merge time'); // the verdict on the seeded record is merge-with-fixes below
|
||||
const result = await bot.scanOnce('test'); // listOpenPrs is mocked to []: 381 is gone
|
||||
expect(result.closed).toEqual([381]);
|
||||
expect(bot.store.pr(381)?.closedAs).toBe('merged');
|
||||
expect(tg.sent.filter((s) => s.text.includes('Merged <b>#381</b>'))).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('ignores a confirmation tap from a foreign chat', async () => {
|
||||
await bot.handleUpdate(msg('/merge 381'));
|
||||
await bot.handleUpdate(cb(tg.confirmData(), 2));
|
||||
expect(gh.mergePr).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('refuses to offer a merge for a conflicting PR and warns about red CI', async () => {
|
||||
gh.getPrDetail.mockImplementationOnce(async () => detail({ mergeable: 'CONFLICTING' }));
|
||||
await bot.handleUpdate(msg('/merge 381'));
|
||||
expect(tg.last()).toContain('needs a rebase');
|
||||
expect(Object.keys(bot.store.state.pending)).toHaveLength(0);
|
||||
|
||||
gh.getCiStatus.mockImplementationOnce(async () => ({ state: 'failed', runs: [] }));
|
||||
await bot.handleUpdate(msg('/merge 381'));
|
||||
expect(tg.last()).toContain('CI is red');
|
||||
expect(Object.keys(bot.store.state.pending)).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('cancel drops the pending confirmation', async () => {
|
||||
await bot.handleUpdate(msg('/merge 381'));
|
||||
const data = tg.confirmData().replace(/^confirm:/, 'cancel:');
|
||||
await bot.handleUpdate(cb(data));
|
||||
expect(Object.keys(bot.store.state.pending)).toHaveLength(0);
|
||||
await bot.handleUpdate(cb(tg.sent[tg.sent.length - 1] ? data.replace(/^cancel:/, 'confirm:') : ''));
|
||||
expect(gh.mergePr).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('closes with the given comment after confirmation, and asks for one when missing', async () => {
|
||||
await bot.handleUpdate(msg('/close 381'));
|
||||
expect(tg.last()).toContain('Reply to this message');
|
||||
expect(gh.closePr).not.toHaveBeenCalled();
|
||||
|
||||
await bot.handleUpdate(msg('/close 381 superseded by #372'));
|
||||
expect(tg.last()).toContain('superseded by #372');
|
||||
await bot.handleUpdate(cb(tg.confirmData()));
|
||||
expect(gh.closePr).toHaveBeenCalledWith('Ark0N/Codeman', 381, 'superseded by #372');
|
||||
});
|
||||
|
||||
it('posts the draft only after confirmation', async () => {
|
||||
await bot.handleUpdate(msg('/post 381'));
|
||||
expect(gh.commentPr).not.toHaveBeenCalled();
|
||||
expect(tg.sent.some((s) => s.plain && s.text === 'Thanks, merging.')).toBe(true);
|
||||
await bot.handleUpdate(cb(tg.confirmData()));
|
||||
expect(gh.commentPr).toHaveBeenCalledWith('Ark0N/Codeman', 381, 'Thanks, merging.');
|
||||
});
|
||||
|
||||
it('a reply to a review message becomes a follow-up, refused when nothing was reviewed', async () => {
|
||||
const rec = bot.store.upsertPr(detail({ number: 390, title: 'other' }));
|
||||
bot.store.rememberMessage(55, 390);
|
||||
expect(rec.reviewedSha).toBeUndefined();
|
||||
await bot.handleUpdate(msg('does it handle X?', 1, { message_id: 55 }));
|
||||
expect(tg.last()).toContain('No review of #390 yet');
|
||||
});
|
||||
|
||||
it('reports status with verdict icons', async () => {
|
||||
await bot.handleUpdate(msg('/status'));
|
||||
expect(tg.last()).toContain('🟢 <b>#381</b>');
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,370 @@
|
||||
/**
|
||||
* @fileoverview Unit tests for the PR bot's pure helpers: report parsing and
|
||||
* Telegram formatting (report.ts), CI classification (github.ts), command and
|
||||
* callback parsing (telegram.ts), the trust-dialog reader (codeman-client.ts) and
|
||||
* config validation (config.ts). No network, no git, no Telegram.
|
||||
*/
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import {
|
||||
buildReportKeyboard,
|
||||
confirmKeyboard,
|
||||
extractJsonObject,
|
||||
formatReviewFailure,
|
||||
formatStatusList,
|
||||
formatTelegramSummary,
|
||||
orderBacklog,
|
||||
parseReport,
|
||||
splitTelegramMessage,
|
||||
TELEGRAM_MAX,
|
||||
type ReviewReport,
|
||||
} from '../scripts/pr-bot/report.js';
|
||||
import { classifyCi, latestRunPerWorkflow, type PrSummary, type WorkflowRun } from '../scripts/pr-bot/github.js';
|
||||
import { parseCallback, parseCommand, prNumberFromMessageText } from '../scripts/pr-bot/telegram.js';
|
||||
import { trustDialogKey } from '../scripts/pr-bot/codeman-client.js';
|
||||
import { buildConfig, parseEnvFile } from '../scripts/pr-bot/config.js';
|
||||
import { buildReviewBrief } from '../scripts/pr-bot/review-task.js';
|
||||
|
||||
const pr: PrSummary = {
|
||||
number: 381,
|
||||
title: 'feat(web): support a reverse-proxy base URL',
|
||||
author: 'mtiller',
|
||||
headSha: '7e4914d991ea864d7dfbefe03f042380d02981c4',
|
||||
baseRef: 'master',
|
||||
headRef: 'feat/reverse-proxy-base-url',
|
||||
isDraft: false,
|
||||
mergeable: 'MERGEABLE',
|
||||
mergeState: 'UNSTABLE',
|
||||
additions: 664,
|
||||
deletions: 112,
|
||||
changedFiles: 28,
|
||||
updatedAt: '2026-09-04T20:15:52Z',
|
||||
url: 'https://github.com/Ark0N/Codeman/pull/381',
|
||||
isCrossRepository: true,
|
||||
labels: [],
|
||||
};
|
||||
|
||||
const rawReport = {
|
||||
verdict: 'request_changes',
|
||||
confidence: 'HIGH',
|
||||
summary: 'Adds a base path. Two real bugs.',
|
||||
changes: ['base-path config', 'ingress rewrite'],
|
||||
findings: [
|
||||
{ severity: 'minor', title: 'nit first in input', file: 'a.ts', line: 1, detail: 'x' },
|
||||
{ severity: 'blocker', title: 'SSE path not prefixed', file: 'src/web/server.ts', line: 210, detail: 'events 404' },
|
||||
{ severity: 'bogus', title: 'unknown severity becomes minor', detail: '' },
|
||||
{ title: '' },
|
||||
],
|
||||
checks: [
|
||||
{ name: 'typecheck', command: 'npm run typecheck', result: 'PASS' },
|
||||
{ name: 'tests', result: 'fail', notes: '2 failed' },
|
||||
{ name: '', result: 'pass' },
|
||||
],
|
||||
scope: 'MIXED',
|
||||
risk: 'r',
|
||||
recommendation: 'Ask for the SSE fix, then merge.',
|
||||
draftComment: 'Thanks!',
|
||||
assumptions: ['none', 42],
|
||||
};
|
||||
|
||||
describe('parseReport', () => {
|
||||
it('normalizes case, separators and severities, and sorts findings by severity', () => {
|
||||
const r = parseReport(rawReport)!;
|
||||
expect(r.verdict).toBe('request-changes');
|
||||
expect(r.confidence).toBe('high');
|
||||
expect(r.scope).toBe('mixed');
|
||||
expect(r.findings.map((f) => f.severity)).toEqual(['blocker', 'minor', 'minor']);
|
||||
expect(r.findings[0].file).toBe('src/web/server.ts');
|
||||
expect(r.findings[0].line).toBe(210);
|
||||
expect(r.checks).toHaveLength(2);
|
||||
expect(r.checks[0].result).toBe('pass');
|
||||
expect(r.assumptions).toEqual(['none']);
|
||||
});
|
||||
|
||||
it('returns null without a recognizable verdict', () => {
|
||||
expect(parseReport({ summary: 'no verdict' })).toBeNull();
|
||||
expect(parseReport(null)).toBeNull();
|
||||
expect(parseReport('merge')).toBeNull();
|
||||
});
|
||||
|
||||
it('defaults confidence to medium', () => {
|
||||
expect(parseReport({ verdict: 'merge' })!.confidence).toBe('medium');
|
||||
});
|
||||
});
|
||||
|
||||
describe('extractJsonObject', () => {
|
||||
it('reads bare JSON, fenced JSON and JSON inside prose', () => {
|
||||
expect(extractJsonObject('{"verdict":"merge"}')).toEqual({ verdict: 'merge' });
|
||||
expect(extractJsonObject('Here:\n```json\n{"verdict":"close"}\n```\nDone.')).toEqual({ verdict: 'close' });
|
||||
expect(extractJsonObject('REVIEW COMPLETE {"verdict":"merge","x":1} trailing')).toEqual({ verdict: 'merge', x: 1 });
|
||||
expect(extractJsonObject('nothing here')).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('formatTelegramSummary', () => {
|
||||
const report = parseReport(rawReport)!;
|
||||
|
||||
it('carries the verdict, the top findings, checks and the recommendation, HTML-escaped', () => {
|
||||
const text = formatTelegramSummary(pr, report, { ci: 'awaiting-approval', durationMin: 7 });
|
||||
expect(text).toContain('PR #381');
|
||||
expect(text).toContain('REQUEST CHANGES');
|
||||
expect(text).toContain('needs your approval');
|
||||
expect(text).toContain('🔴 SSE path not prefixed');
|
||||
expect(text).toContain('src/web/server.ts:210');
|
||||
expect(text).toContain('typecheck ✅');
|
||||
expect(text).toContain('tests ❌');
|
||||
expect(text).toContain('Ask for the SSE fix');
|
||||
expect(text).toContain('review took 7 min');
|
||||
expect(text.length).toBeLessThan(TELEGRAM_MAX);
|
||||
});
|
||||
|
||||
it('escapes HTML in model output', () => {
|
||||
const r: ReviewReport = { ...report, summary: 'uses <script> & friends', findings: [] };
|
||||
const text = formatTelegramSummary(pr, r, { ci: 'passed' });
|
||||
expect(text).toContain('uses <script> & friends');
|
||||
expect(text).not.toContain('<script>');
|
||||
});
|
||||
|
||||
it('stays under the Telegram cap with many long findings and says how many are hidden', () => {
|
||||
const findings = Array.from({ length: 60 }, (_, i) => ({
|
||||
severity: 'major' as const,
|
||||
title: `finding ${i} ${'x'.repeat(150)}`,
|
||||
file: `src/file-${i}.ts`,
|
||||
line: i,
|
||||
detail: 'd',
|
||||
}));
|
||||
const text = formatTelegramSummary(pr, { ...report, findings }, { ci: 'failed' });
|
||||
expect(text.length).toBeLessThanOrEqual(TELEGRAM_MAX);
|
||||
expect(text).toMatch(/… \d+ more in the full report/);
|
||||
});
|
||||
});
|
||||
|
||||
describe('splitTelegramMessage', () => {
|
||||
it('keeps short text whole and splits long text on line boundaries', () => {
|
||||
expect(splitTelegramMessage('a\nb')).toEqual(['a\nb']);
|
||||
const lines = Array.from({ length: 300 }, (_, i) => `line ${i} ${'y'.repeat(40)}`);
|
||||
const chunks = splitTelegramMessage(lines.join('\n'));
|
||||
expect(chunks.length).toBeGreaterThan(1);
|
||||
for (const c of chunks) expect(c.length).toBeLessThanOrEqual(TELEGRAM_MAX);
|
||||
expect(chunks.join('\n')).toBe(lines.join('\n'));
|
||||
});
|
||||
|
||||
it('hard-splits a single line longer than the cap', () => {
|
||||
const chunks = splitTelegramMessage('z'.repeat(9000), 4000);
|
||||
expect(chunks.map((c) => c.length)).toEqual([4000, 4000, 1000]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('keyboards', () => {
|
||||
it("keeps every callback_data under Telegram's 64-byte cap and adds the approve button only when CI waits", () => {
|
||||
const all = [
|
||||
...buildReportKeyboard(38199, { ci: 'awaiting-approval', hasDraft: true }).flat(),
|
||||
...buildReportKeyboard(1, { ci: 'passed', hasDraft: false }).flat(),
|
||||
...confirmKeyboard('merge', 38199, 'deadbeef').flat(),
|
||||
];
|
||||
for (const b of all) expect(Buffer.byteLength(b.callback_data)).toBeLessThanOrEqual(64);
|
||||
expect(
|
||||
buildReportKeyboard(5, { ci: 'awaiting-approval', hasDraft: true })
|
||||
.flat()
|
||||
.some((b) => b.callback_data === 'approveci:5')
|
||||
).toBe(true);
|
||||
expect(
|
||||
buildReportKeyboard(5, { ci: 'passed', hasDraft: true })
|
||||
.flat()
|
||||
.some((b) => b.callback_data.startsWith('approveci'))
|
||||
).toBe(false);
|
||||
expect(
|
||||
buildReportKeyboard(5, { ci: 'passed', hasDraft: false })
|
||||
.flat()
|
||||
.some((b) => b.callback_data === 'post:5')
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('orderBacklog', () => {
|
||||
it('puts mergeable and small first, conflicting last, newer first on ties', () => {
|
||||
const rows = [
|
||||
{ number: 375, mergeable: 'CONFLICTING' as const, additions: 5000, deletions: 100 },
|
||||
{ number: 383, mergeable: 'MERGEABLE' as const, additions: 29, deletions: 1 },
|
||||
{ number: 380, mergeable: 'MERGEABLE' as const, additions: 1800, deletions: 400 },
|
||||
{ number: 362, mergeable: 'CONFLICTING' as const, additions: 200, deletions: 10 },
|
||||
{ number: 390, mergeable: 'UNKNOWN' as const, additions: 29, deletions: 1 },
|
||||
];
|
||||
expect(orderBacklog(rows).map((r) => r.number)).toEqual([390, 383, 380, 362, 375]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('formatStatusList + formatReviewFailure', () => {
|
||||
it('renders rows with verdict icons and flags', () => {
|
||||
const text = formatStatusList(
|
||||
[
|
||||
{
|
||||
number: 1,
|
||||
title: 'a',
|
||||
author: 'x',
|
||||
verdict: 'merge',
|
||||
status: 'reviewed',
|
||||
ci: 'passed',
|
||||
mergeable: 'MERGEABLE',
|
||||
isDraft: false,
|
||||
},
|
||||
{ number: 2, title: 'b <c>', author: 'y', status: 'queued', mergeable: 'CONFLICTING', isDraft: true },
|
||||
],
|
||||
true
|
||||
);
|
||||
expect(text).toContain('⏸ auto-review paused');
|
||||
expect(text).toContain('✅ <b>#1</b>');
|
||||
expect(text).toContain('🕓 <b>#2</b> b <c>');
|
||||
expect(text).toContain('conflicts, draft');
|
||||
expect(formatStatusList([], false)).toBe('No open pull requests.');
|
||||
});
|
||||
|
||||
it('failure notice names the retry command', () => {
|
||||
expect(formatReviewFailure(pr, 'timed out')).toContain('/review 381');
|
||||
});
|
||||
});
|
||||
|
||||
describe('classifyCi', () => {
|
||||
const run = (over: Partial<WorkflowRun>): WorkflowRun => ({
|
||||
id: 1,
|
||||
name: 'CI',
|
||||
status: 'completed',
|
||||
conclusion: 'success',
|
||||
...over,
|
||||
});
|
||||
|
||||
it('reads the newest run per workflow only', () => {
|
||||
const runs = [
|
||||
run({ id: 3, conclusion: 'success' }),
|
||||
run({ id: 2, conclusion: 'failure' }),
|
||||
run({ id: 1, name: 'Other', conclusion: 'failure' }),
|
||||
];
|
||||
expect(latestRunPerWorkflow(runs).map((r) => r.id)).toEqual([3, 1]);
|
||||
expect(classifyCi(runs)).toBe('failed');
|
||||
expect(classifyCi([run({ id: 3 }), run({ id: 2, conclusion: 'failure' })])).toBe('passed');
|
||||
});
|
||||
|
||||
it('maps the fork-PR approval gate, pending and empty cases', () => {
|
||||
expect(classifyCi([])).toBe('none');
|
||||
expect(classifyCi([run({ conclusion: 'action_required' })])).toBe('awaiting-approval');
|
||||
expect(classifyCi([run({ status: 'in_progress', conclusion: null })])).toBe('pending');
|
||||
expect(classifyCi([run({ conclusion: 'skipped' })])).toBe('passed');
|
||||
});
|
||||
});
|
||||
|
||||
describe('telegram parsers', () => {
|
||||
it('parses commands with and without a PR number, and bot-suffixed commands', () => {
|
||||
expect(parseCommand('/merge 381')).toEqual({ command: 'merge', prNumber: 381, rest: '' });
|
||||
expect(parseCommand('/close #381 superseded by #372')).toEqual({
|
||||
command: 'close',
|
||||
prNumber: 381,
|
||||
rest: 'superseded by #372',
|
||||
});
|
||||
expect(parseCommand('/ask 12 does it handle\nmultiline?')).toEqual({
|
||||
command: 'ask',
|
||||
prNumber: 12,
|
||||
rest: 'does it handle\nmultiline?',
|
||||
});
|
||||
expect(parseCommand('/status@arkon85_bot')).toEqual({ command: 'status', rest: '' });
|
||||
expect(parseCommand('hello')).toBeNull();
|
||||
expect(parseCommand(undefined)).toBeNull();
|
||||
});
|
||||
|
||||
it('parses callbacks and rejects malformed data', () => {
|
||||
expect(parseCallback('merge:381')).toEqual({ action: 'merge', prNumber: 381 });
|
||||
expect(parseCallback('confirm:merge:381:ab12')).toEqual({
|
||||
action: 'confirm',
|
||||
target: 'merge',
|
||||
prNumber: 381,
|
||||
nonce: 'ab12',
|
||||
});
|
||||
expect(parseCallback('confirm:merge:381')).toBeNull();
|
||||
expect(parseCallback('merge:x')).toBeNull();
|
||||
expect(parseCallback(undefined)).toBeNull();
|
||||
});
|
||||
|
||||
it('finds the PR number in a report message', () => {
|
||||
expect(prNumberFromMessageText('🔍 PR #381 · title')).toBe(381);
|
||||
expect(prNumberFromMessageText('no number')).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('trustDialogKey', () => {
|
||||
it('reads the highlighted option off a tmux repaint that lost its spaces', () => {
|
||||
const esc = '\x1b';
|
||||
const screen = `Security guide\n${esc}[1m❯${esc}[CNo,${esc}[Cexit\n Yes, I trust this folder\nEnter to confirm`;
|
||||
expect(trustDialogKey(screen)).toBe('move');
|
||||
expect(trustDialogKey(' No, exit\n❯ Yes, I trust this folder\n')).toBe('confirm');
|
||||
expect(trustDialogKey('❯ Try "fix the bug"\n shift+tab to cycle')).toBeNull();
|
||||
});
|
||||
|
||||
it('lets the freshest marked row win', () => {
|
||||
expect(trustDialogKey('❯ No, exit\n...\n❯ Yes, I trust this folder')).toBe('confirm');
|
||||
});
|
||||
});
|
||||
|
||||
describe('config', () => {
|
||||
it('parses env files with quotes, comments and export prefixes', () => {
|
||||
const env = parseEnvFile('# c\nexport A="x y"\nB=\'z\'\nC=plain\nbad line\n=nokey\n');
|
||||
expect(env).toEqual({ A: 'x y', B: 'z', C: 'plain' });
|
||||
});
|
||||
|
||||
it('validates required keys and derives paths and units', () => {
|
||||
expect(() => buildConfig({}, { home: '/h', repoRoot: '/r' })).toThrow(/TELEGRAM_BOT_TOKEN, TELEGRAM_CHAT_ID/);
|
||||
const cfg = buildConfig(
|
||||
{
|
||||
TELEGRAM_BOT_TOKEN: 't',
|
||||
TELEGRAM_CHAT_ID: '1',
|
||||
PR_BOT_POLL_INTERVAL: '30',
|
||||
PR_BOT_REVIEW_TIMEOUT: '3',
|
||||
PR_BOT_AUTO_REVIEW: 'off',
|
||||
},
|
||||
{ home: '/h', repoRoot: '/r' }
|
||||
);
|
||||
expect(cfg.githubRepo).toBe('Ark0N/Codeman');
|
||||
expect(cfg.codemanApiUrl).toBe('https://127.0.0.1:3000');
|
||||
expect(cfg.dataDir).toBe('/h/.codeman/pr-bot');
|
||||
expect(cfg.worktreesDir).toBe('/h/.codeman/pr-bot/worktrees');
|
||||
expect(cfg.mainCheckout).toBe('/r');
|
||||
expect(cfg.pollIntervalMs).toBe(60_000); // floored at 60s
|
||||
expect(cfg.reviewTimeoutMs).toBe(5 * 60_000); // floored at 5 min
|
||||
expect(cfg.autoReview).toBe(false);
|
||||
expect(cfg.reviewDrafts).toBe(false);
|
||||
expect(() =>
|
||||
buildConfig(
|
||||
{ TELEGRAM_BOT_TOKEN: 't', TELEGRAM_CHAT_ID: '1', GITHUB_REPO: 'nope' },
|
||||
{ home: '/h', repoRoot: '/r' }
|
||||
)
|
||||
).toThrow(/owner\/name/);
|
||||
});
|
||||
});
|
||||
|
||||
describe('buildReviewBrief', () => {
|
||||
it('names the report paths, the ground rules and the CI situation', () => {
|
||||
const brief = buildReviewBrief({
|
||||
pr: {
|
||||
...pr,
|
||||
body: 'Body **md**',
|
||||
files: [{ path: 'src/a.ts', additions: 1, deletions: 0 }],
|
||||
authorAssociation: 'FIRST_TIME_CONTRIBUTOR',
|
||||
linkedIssues: [],
|
||||
commitCount: 2,
|
||||
commentCount: 0,
|
||||
reviewDecision: '',
|
||||
headRepo: 'mtiller/Codeman',
|
||||
},
|
||||
ci: { state: 'awaiting-approval', runs: [] },
|
||||
mergeBase: 'abcdef0123456789',
|
||||
worktreeDir: '/wt/pr-381',
|
||||
mainCheckout: '/main',
|
||||
reportJsonPath: '/jobs/report.json',
|
||||
reportMdPath: '/jobs/report.md',
|
||||
});
|
||||
expect(brief).toContain('/jobs/report.json');
|
||||
expect(brief).toContain('/jobs/report.md');
|
||||
expect(brief).toContain('REVIEW COMPLETE');
|
||||
expect(brief).toContain('waiting for a maintainer to approve');
|
||||
expect(brief).toContain('never bind port 3000');
|
||||
expect(brief).toContain('first time contributor');
|
||||
expect(brief).toContain('`src/a.ts` (+1/-0)');
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,84 @@
|
||||
/**
|
||||
* @fileoverview StateStore semantics for the PR bot: upsert keeps review results
|
||||
* across scans, a closed PR that reopens comes back as reviewed, saves are atomic
|
||||
* and 0600, and the message map is bounded.
|
||||
*/
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { mkdtempSync, readdirSync, statSync } from 'fs';
|
||||
import { tmpdir } from 'os';
|
||||
import { join } from 'path';
|
||||
import { StateStore } from '../scripts/pr-bot/state.js';
|
||||
import type { PrSummary } from '../scripts/pr-bot/github.js';
|
||||
|
||||
function summary(over: Partial<PrSummary> = {}): PrSummary {
|
||||
return {
|
||||
number: 7,
|
||||
title: 't',
|
||||
author: 'a',
|
||||
headSha: 'aaaa',
|
||||
baseRef: 'master',
|
||||
headRef: 'x',
|
||||
isDraft: false,
|
||||
mergeable: 'MERGEABLE',
|
||||
mergeState: 'CLEAN',
|
||||
additions: 1,
|
||||
deletions: 1,
|
||||
changedFiles: 1,
|
||||
updatedAt: '',
|
||||
url: 'https://example/7',
|
||||
isCrossRepository: true,
|
||||
labels: [],
|
||||
...over,
|
||||
};
|
||||
}
|
||||
|
||||
describe('StateStore', () => {
|
||||
it('starts empty, persists, and reloads', () => {
|
||||
const dir = mkdtempSync(join(tmpdir(), 'prbot-state-'));
|
||||
const path = join(dir, 'state.json');
|
||||
const store = new StateStore(path);
|
||||
const rec = store.upsertPr(summary());
|
||||
expect(rec.status).toBe('new');
|
||||
rec.status = 'reviewed';
|
||||
rec.reviewedSha = 'aaaa';
|
||||
rec.verdict = 'merge';
|
||||
store.state.telegramOffset = 42;
|
||||
store.save();
|
||||
expect(statSync(path).mode & 0o777).toBe(0o600);
|
||||
expect(readdirSync(dir)).toEqual(['state.json']); // no tmp file left behind
|
||||
const again = new StateStore(path);
|
||||
expect(again.pr(7)?.verdict).toBe('merge');
|
||||
expect(again.state.telegramOffset).toBe(42);
|
||||
});
|
||||
|
||||
it('upsert refreshes metadata but keeps the review; reopening a closed PR restores reviewed', () => {
|
||||
const store = new StateStore(join(mkdtempSync(join(tmpdir(), 'prbot-state-')), 'state.json'));
|
||||
const rec = store.upsertPr(summary());
|
||||
rec.status = 'reviewed';
|
||||
rec.reviewedSha = 'aaaa';
|
||||
const moved = store.upsertPr(summary({ headSha: 'bbbb', title: 'renamed' }));
|
||||
expect(moved).toBe(rec); // same object: a review in flight keeps writing into the stored record
|
||||
expect(moved.status).toBe('reviewed');
|
||||
expect(moved.reviewedSha).toBe('aaaa');
|
||||
expect(moved.headSha).toBe('bbbb');
|
||||
expect(moved.title).toBe('renamed');
|
||||
moved.status = 'closed';
|
||||
moved.closedAs = 'closed';
|
||||
expect(store.openPrs()).toHaveLength(0);
|
||||
const reopened = store.upsertPr(summary({ headSha: 'bbbb' }));
|
||||
expect(reopened.status).toBe('reviewed');
|
||||
expect(reopened.closedAs).toBeUndefined();
|
||||
expect(store.openPrs()).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('bounds the message map on save', () => {
|
||||
const store = new StateStore(join(mkdtempSync(join(tmpdir(), 'prbot-state-')), 'state.json'));
|
||||
for (let i = 0; i < 2500; i++) store.rememberMessage(i, 1);
|
||||
store.save();
|
||||
const keys = Object.keys(store.state.messages).map(Number);
|
||||
expect(keys).toHaveLength(2000);
|
||||
expect(Math.min(...keys)).toBe(500);
|
||||
expect(store.prForMessage(2499)).toBe(1);
|
||||
expect(store.prForMessage(10)).toBeUndefined();
|
||||
});
|
||||
});
|
||||
@@ -228,3 +228,42 @@ describe('WebServer.renderIndexHtml', () => {
|
||||
expect(html).not.toContain('gesture-codeman.js');
|
||||
});
|
||||
});
|
||||
|
||||
describe('WebServer.renderIndexHtml reverse-proxy base path', () => {
|
||||
const BASE_TEMPLATE = ['<head>', '<base href="/">', '<title>Codeman</title>', '</head>', '<body></body>'].join('\n');
|
||||
|
||||
function makeBaseServer(basePath: string) {
|
||||
// constructor: (port, https, testMode, host, titleHostname, allowUnauth, basePath)
|
||||
const server = new WebServer(0, false, true, '127.0.0.1', undefined, false, basePath);
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
(server as any).indexHtmlTemplate = BASE_TEMPLATE;
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
(server as any).readSettings = vi.fn(async () => ({}));
|
||||
return server;
|
||||
}
|
||||
|
||||
it('is inert at root — base tag unchanged and no base global injected', async () => {
|
||||
const server = makeBaseServer('');
|
||||
const html = await render(server);
|
||||
expect(html).toContain('<base href="/">');
|
||||
// At root the frontend reads a MISSING __CODEMAN_BASE__ as root, so nothing is
|
||||
// injected and the historical output is byte-identical.
|
||||
expect(html).not.toContain('__CODEMAN_BASE__');
|
||||
});
|
||||
|
||||
it('points the base tag and the base global at a sub-path mount', async () => {
|
||||
const server = makeBaseServer('/codeman');
|
||||
const html = await render(server);
|
||||
expect(html).toContain('<base href="/codeman/">');
|
||||
expect(html).toContain('window.__CODEMAN_BASE__="/codeman"');
|
||||
// The global rides right after <base>, before any (deferred) script.
|
||||
expect(html.indexOf('window.__CODEMAN_BASE__')).toBeLessThan(html.indexOf('</head>'));
|
||||
});
|
||||
|
||||
it('normalizes a raw operator prefix passed to the constructor', async () => {
|
||||
const server = makeBaseServer('codeman/');
|
||||
const html = await render(server);
|
||||
expect(html).toContain('<base href="/codeman/">');
|
||||
expect(html).toContain('window.__CODEMAN_BASE__="/codeman"');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,149 @@
|
||||
/**
|
||||
* @fileoverview Response-viewer turn segmentation (`CodemanApp._buildResponseViewerMessage`).
|
||||
*
|
||||
* The server now emits one message per model message instead of concatenating a
|
||||
* human turn's replies into one card, so a long autonomous run arrives as tens
|
||||
* of messages rather than one 12,000-character block. Rendered naively that is
|
||||
* card spam — the measured distribution is p50 3 messages per turn, p90 11,
|
||||
* max 51, with 58% of messages under 80 characters. So consecutive messages
|
||||
* from one speaker inside one `turn` render as SEGMENTS of one card: no
|
||||
* repeated role badge, a hairline seam.
|
||||
*
|
||||
* Pinned here because the badge suppression is the only thing standing between
|
||||
* the server change and a wall of 51 "Claude" badges:
|
||||
*
|
||||
* 1. A continuation carries `rv-msg-cont` and has NO `.rv-role` child, while
|
||||
* keeping its role class so the CSS accent survives (the colour rules match
|
||||
* on both `:has(.rv-role-*)` and `.rv-msg-*` — only the class arm hits here).
|
||||
* 2. A queued prompt is marked in the DOM, not in text, so the i18n
|
||||
* MutationObserver cannot rewrite the marker.
|
||||
* 3. The 4th argument is genuinely optional: the brief view's 3-argument call
|
||||
* still renders a badge.
|
||||
*
|
||||
* Loaded via `vm` with a jsdom document injected (same technique as
|
||||
* response-viewer-file-links.test.ts).
|
||||
* Port: N/A
|
||||
*/
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { performance } from 'node:perf_hooks';
|
||||
import { resolve } from 'node:path';
|
||||
import vm from 'node:vm';
|
||||
import { JSDOM } from 'jsdom';
|
||||
import { describe, expect, it, vi } from 'vitest';
|
||||
|
||||
const dom = new JSDOM('<!DOCTYPE html><html><body></body></html>');
|
||||
const { document, NodeFilter } = dom.window;
|
||||
|
||||
interface MessageBuilder {
|
||||
_buildResponseViewerMessage(text: string, role: string, agentLabel: string, meta?: unknown): HTMLElement;
|
||||
loadFullContext(): Promise<void>;
|
||||
activeSessionId?: string;
|
||||
}
|
||||
|
||||
function loadCodemanAppClass() {
|
||||
const constants = readFileSync(resolve(import.meta.dirname, '../src/web/public/constants.js'), 'utf8');
|
||||
const source = readFileSync(resolve(import.meta.dirname, '../src/web/public/app.js'), 'utf8');
|
||||
const context = vm.createContext({
|
||||
console,
|
||||
performance,
|
||||
setInterval: vi.fn(),
|
||||
clearInterval: vi.fn(),
|
||||
setTimeout,
|
||||
clearTimeout,
|
||||
requestAnimationFrame: vi.fn(),
|
||||
HTMLCanvasElement: class HTMLCanvasElement {},
|
||||
fetch: vi.fn(),
|
||||
document,
|
||||
NodeFilter,
|
||||
localStorage: { length: 0, key: vi.fn(), getItem: vi.fn(), setItem: vi.fn(), removeItem: vi.fn() },
|
||||
window: { addEventListener: vi.fn(), removeEventListener: vi.fn() },
|
||||
MobileDetection: {},
|
||||
});
|
||||
vm.runInContext(`${constants}\n${source}\nglobalThis.__CodemanApp = CodemanApp;`, context);
|
||||
// The context is returned too: app.js closes over the context's own `fetch`, so a
|
||||
// test that drives loadFullContext has to replace THAT binding, not globalThis'.
|
||||
return { CodemanApp: (context as { __CodemanApp: { prototype: MessageBuilder } }).__CodemanApp, context };
|
||||
}
|
||||
|
||||
const { CodemanApp, context: appContext } = loadCodemanAppClass();
|
||||
|
||||
function build(text: string, role: string, meta?: unknown): HTMLElement {
|
||||
const app = Object.create(CodemanApp.prototype) as MessageBuilder;
|
||||
return app._buildResponseViewerMessage(text, role, 'Claude', meta);
|
||||
}
|
||||
|
||||
describe('response viewer turn segmentation', () => {
|
||||
it('renders a continuation without a repeated role badge but keeps its role class', () => {
|
||||
const div = build('second half of the same turn', 'assistant', { continuation: true, kind: 'response', turn: 3 });
|
||||
|
||||
expect(div.classList.contains('rv-msg-cont')).toBe(true);
|
||||
expect(div.classList.contains('rv-msg-assistant')).toBe(true);
|
||||
expect(div.querySelector('.rv-role')).toBeNull();
|
||||
expect(div.querySelector('.rv-text')).not.toBeNull();
|
||||
expect(div.dataset.kind).toBe('response');
|
||||
});
|
||||
|
||||
it('marks a prompt the user queued mid-turn in the DOM, not in the text', () => {
|
||||
const div = build('actually use PowerShell', 'user', {
|
||||
continuation: false,
|
||||
kind: 'prompt',
|
||||
queued: true,
|
||||
turn: 2,
|
||||
});
|
||||
|
||||
expect(div.dataset.queued).toBe('1');
|
||||
expect(div.dataset.kind).toBe('prompt');
|
||||
const badge = div.querySelector('.rv-role');
|
||||
expect(badge).not.toBeNull();
|
||||
expect(badge!.classList.contains('rv-role-user')).toBe(true);
|
||||
// The marker is a CSS pseudo-element, so the badge text stays translatable.
|
||||
expect(badge!.textContent).toBe('You');
|
||||
});
|
||||
|
||||
it('still renders a badge for the brief view, which passes no meta', () => {
|
||||
const div = build('the last response', 'assistant');
|
||||
|
||||
expect(div.classList.contains('rv-msg-cont')).toBe(false);
|
||||
expect(div.querySelector('.rv-role')!.textContent).toBe('Claude');
|
||||
expect(div.dataset.kind).toBeUndefined();
|
||||
expect(div.dataset.queued).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* The empty-state branch deliberately does NOT wipe the body — the brief view has
|
||||
* a terminal-buffer fallback this endpoint does not — and deliberately leaves the
|
||||
* More button live so a transcript that appears a moment later can still be
|
||||
* loaded. Both together mean the notice must be idempotent: without that, every
|
||||
* retry stacks another identical line. Upstream got this for free because it
|
||||
* assigned `body.textContent`.
|
||||
*/
|
||||
describe('response viewer empty full-context state', () => {
|
||||
it('reuses one notice across repeated More clicks and keeps the brief card', async () => {
|
||||
const body = document.createElement('div');
|
||||
body.id = 'responseViewerBody';
|
||||
const title = document.createElement('div');
|
||||
title.id = 'responseViewerTitle';
|
||||
const more = document.createElement('button');
|
||||
more.id = 'responseViewerMore';
|
||||
document.body.append(body, title, more);
|
||||
body.textContent = 'No response yet — send a message in this session first.';
|
||||
|
||||
const app = Object.create(CodemanApp.prototype) as MessageBuilder;
|
||||
app.activeSessionId = 's1';
|
||||
(appContext as { fetch: unknown }).fetch = vi.fn(async () => ({
|
||||
json: async () => ({ data: { messages: [] } }),
|
||||
}));
|
||||
|
||||
await app.loadFullContext();
|
||||
await app.loadFullContext();
|
||||
await app.loadFullContext();
|
||||
|
||||
expect(body.querySelectorAll('.rv-notice')).toHaveLength(1);
|
||||
expect(body.textContent).toContain('No response yet');
|
||||
// More stays clickable: it is the only retry path once a transcript lands.
|
||||
expect(more.style.display).toBe('');
|
||||
|
||||
document.body.innerHTML = '';
|
||||
});
|
||||
});
|
||||
@@ -10,6 +10,7 @@ import { Readable } from 'node:stream';
|
||||
import { createRouteTestHarness, type RouteTestHarness } from './_route-test-utils.js';
|
||||
import { registerFileRoutes } from '../../src/web/routes/file-routes.js';
|
||||
import { ApiErrorCode } from '../../src/types.js';
|
||||
import { CASES_DIR } from '../../src/web/route-helpers.js';
|
||||
|
||||
// Mock fs/promises for file operations
|
||||
vi.mock('node:fs/promises', () => ({
|
||||
@@ -114,6 +115,20 @@ describe('file-routes', () => {
|
||||
]);
|
||||
});
|
||||
|
||||
it('defaults to the Codeman Cases root, not Home, when linking a case with no path chosen yet', async () => {
|
||||
// The "Link Existing" case picker opens with an empty path and no
|
||||
// sessionId. `Home` and `Codeman Cases` are unrelated bind mounts under
|
||||
// Docker, so falling back to whichever root happened to be listed first
|
||||
// could open the picker somewhere with no cases in it at all — and, worse,
|
||||
// make a stale directory from a since-changed CODEMAN_CASES_PATH look like
|
||||
// a normal thing to stumble across while browsing for one to link.
|
||||
const res = await harness.app.inject({ method: 'GET', url: '/api/filesystem/browse' });
|
||||
|
||||
expect(res.statusCode).toBe(200);
|
||||
const body = JSON.parse(res.body);
|
||||
expect(body.data.path).toBe(CASES_DIR);
|
||||
});
|
||||
|
||||
it('rejects paths outside the configured roots', async () => {
|
||||
const res = await harness.app.inject({
|
||||
method: 'GET',
|
||||
|
||||
@@ -115,6 +115,75 @@ describe('hook-event-routes', () => {
|
||||
);
|
||||
});
|
||||
|
||||
/**
|
||||
* The pane's live conversation id, reported by the CLI process itself. This
|
||||
* is what lets the response viewer stop guessing from ~/.claude/history.jsonl
|
||||
* — a guess that could never run at all for a pane the user drives by
|
||||
* attaching to tmux, because `lastSubmitAt` only ever saw Codeman's own
|
||||
* write path.
|
||||
*/
|
||||
it('adopts the conversation id first-hand from a prompt_submitted hook', async () => {
|
||||
const session = harness.ctx._session;
|
||||
const before = session.lastSubmitAt;
|
||||
|
||||
const res = await harness.app.inject({
|
||||
method: 'POST',
|
||||
url: '/api/hook-event',
|
||||
payload: {
|
||||
event: 'prompt_submitted',
|
||||
sessionId: harness.ctx._sessionId,
|
||||
data: { hook_event_name: 'UserPromptSubmit', session_id: 'conv-1', source: 'user' },
|
||||
},
|
||||
});
|
||||
|
||||
expect(res.statusCode).toBe(200);
|
||||
expect(session.claudeSessionId).toBe('conv-1');
|
||||
expect(session.claudeSessionIdIsFirstHand).toBe(true);
|
||||
expect(session.claudeSessionChain).toEqual(['conv-1']);
|
||||
expect(session.lastSubmitAt).toBeGreaterThan(before);
|
||||
expect(harness.ctx.persistSessionState).toHaveBeenCalledWith(session);
|
||||
});
|
||||
|
||||
it('records a /clear successor in the chain and persists it, without duplicating a repeat', async () => {
|
||||
const session = harness.ctx._session;
|
||||
const submit = async (conversationId: string) =>
|
||||
harness.app.inject({
|
||||
method: 'POST',
|
||||
url: '/api/hook-event',
|
||||
payload: {
|
||||
event: 'prompt_submitted',
|
||||
sessionId: harness.ctx._sessionId,
|
||||
data: { hook_event_name: 'UserPromptSubmit', session_id: conversationId },
|
||||
},
|
||||
});
|
||||
|
||||
await submit('conv-1');
|
||||
await submit('conv-1'); // every prompt in a conversation reports the same id
|
||||
await submit('conv-2'); // the user ran /clear
|
||||
|
||||
expect(session.claudeSessionChain).toEqual(['conv-1', 'conv-2']);
|
||||
expect(session.claudeSessionId).toBe('conv-2');
|
||||
// `/clear` emits no completion event, so the successor is lost on restart
|
||||
// unless the hook itself persists it.
|
||||
expect(harness.ctx.persistSessionState).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it('does not leak the prompt text into the broadcast', async () => {
|
||||
await harness.app.inject({
|
||||
method: 'POST',
|
||||
url: '/api/hook-event',
|
||||
payload: {
|
||||
event: 'prompt_submitted',
|
||||
sessionId: harness.ctx._sessionId,
|
||||
data: { hook_event_name: 'UserPromptSubmit', session_id: 'conv-1', prompt: 'my secret prompt' },
|
||||
},
|
||||
});
|
||||
|
||||
const broadcast = JSON.stringify(harness.ctx.broadcast.mock.calls);
|
||||
expect(broadcast).not.toContain('my secret prompt');
|
||||
expect(broadcast).toContain('conv-1');
|
||||
});
|
||||
|
||||
it('returns 404 for unknown session', async () => {
|
||||
const res = await harness.app.inject({
|
||||
method: 'POST',
|
||||
|
||||
@@ -60,6 +60,24 @@ const assistantEntry = (text: string, timestamp: string) => ({
|
||||
message: { content: [{ type: 'text', text }] },
|
||||
});
|
||||
|
||||
/**
|
||||
* A prompt typed while Claude is working. Shape copied from a real CLI 2.1.251
|
||||
* row: the CLI's own queue entries carry commandMode 'task-notification' and no
|
||||
* `origin` key at all, which is what separates them from the human's.
|
||||
*/
|
||||
const queuedEntry = (prompt: string, timestamp: string, kind: 'human' | 'task-notification' = 'human') => ({
|
||||
type: 'attachment',
|
||||
timestamp,
|
||||
attachment: {
|
||||
type: 'queued_command',
|
||||
prompt,
|
||||
source_uuid: `src-${timestamp}`,
|
||||
commandMode: kind === 'human' ? 'prompt' : 'task-notification',
|
||||
...(kind === 'human' ? { origin: { kind: 'human' } } : {}),
|
||||
timestamp,
|
||||
},
|
||||
});
|
||||
|
||||
describe('GET /api/sessions/:id/last-response (claude)', () => {
|
||||
let harness: LocalHarness;
|
||||
let testHome: string;
|
||||
@@ -93,7 +111,7 @@ describe('GET /api/sessions/:id/last-response (claude)', () => {
|
||||
return { response, body: JSON.parse(response.body) };
|
||||
}
|
||||
|
||||
it('recovers a placeholder tmux session by UUID prefix and groups JSONL fragments into turns', async () => {
|
||||
it('recovers a placeholder tmux session by UUID prefix and renders one message per model message', async () => {
|
||||
const restoredId = 'restored-40568a29';
|
||||
const conversationId = '40568a29-d4eb-4eb6-b671-8401428e4f39';
|
||||
const session = harness.ctx._session as typeof harness.ctx._session & {
|
||||
@@ -135,15 +153,60 @@ describe('GET /api/sessions/:id/last-response (claude)', () => {
|
||||
expect(full.body.data).toEqual({
|
||||
text: 'Second half.',
|
||||
timestamp: '2026-07-21T00:00:06Z',
|
||||
// #169's guarantees all still hold and this array proves them: the replayed
|
||||
// 'first prompt' row, the replayed 'Checking the files.' snapshot, the
|
||||
// sidechain row and all five synthetic rows are absent. Only the GROUPING
|
||||
// UNIT narrows, from one card per human turn to one card per model
|
||||
// message, carried by `turn` instead of by a '\n\n' joiner.
|
||||
messages: [
|
||||
{ role: 'user', text: 'first prompt', timestamp: '2026-07-21T00:00:00Z' },
|
||||
{
|
||||
kind: 'prompt',
|
||||
label: 'Prompt',
|
||||
role: 'user',
|
||||
text: 'first prompt',
|
||||
timestamp: '2026-07-21T00:00:00Z',
|
||||
turn: 1,
|
||||
},
|
||||
{
|
||||
kind: 'response',
|
||||
label: 'Response',
|
||||
role: 'assistant',
|
||||
text: 'Checking the files.\n\nThe first result is ready.',
|
||||
text: 'Checking the files.',
|
||||
timestamp: '2026-07-21T00:00:01Z',
|
||||
turn: 1,
|
||||
},
|
||||
{
|
||||
kind: 'response',
|
||||
label: 'Response',
|
||||
role: 'assistant',
|
||||
text: 'The first result is ready.',
|
||||
timestamp: '2026-07-21T00:00:03Z',
|
||||
turn: 1,
|
||||
},
|
||||
{
|
||||
kind: 'prompt',
|
||||
label: 'Prompt',
|
||||
role: 'user',
|
||||
text: 'second prompt',
|
||||
timestamp: '2026-07-21T00:00:00Z',
|
||||
turn: 2,
|
||||
},
|
||||
{
|
||||
kind: 'response',
|
||||
label: 'Response',
|
||||
role: 'assistant',
|
||||
text: 'First half.',
|
||||
timestamp: '2026-07-21T00:00:04Z',
|
||||
turn: 2,
|
||||
},
|
||||
{
|
||||
kind: 'response',
|
||||
label: 'Response',
|
||||
role: 'assistant',
|
||||
text: 'Second half.',
|
||||
timestamp: '2026-07-21T00:00:06Z',
|
||||
turn: 2,
|
||||
},
|
||||
{ role: 'user', text: 'second prompt', timestamp: '2026-07-21T00:00:00Z' },
|
||||
{ role: 'assistant', text: 'First half.\n\nSecond half.', timestamp: '2026-07-21T00:00:06Z' },
|
||||
],
|
||||
});
|
||||
expect(session.adoptClaudeSessionId).toHaveBeenCalledWith(conversationId);
|
||||
@@ -175,6 +238,137 @@ describe('GET /api/sessions/:id/last-response (claude)', () => {
|
||||
['assistant', 'Second answer.'],
|
||||
]);
|
||||
});
|
||||
|
||||
/**
|
||||
* A prompt typed while Claude is working is absorbed mid-turn and recorded
|
||||
* ONLY as an attachment row — 160 of the 347 user cards across a real
|
||||
* ~/.claude/projects. Reading only `user` rows lost them outright AND lost the
|
||||
* turn boundary they carry, which is what let an assistant run fuse.
|
||||
*/
|
||||
it('surfaces a prompt the user queued while Claude was working', async () => {
|
||||
const sessionId = harness.ctx._session.id;
|
||||
const session = harness.ctx._session as typeof harness.ctx._session & {
|
||||
claudeSessionId: string;
|
||||
adoptClaudeSessionId: ReturnType<typeof vi.fn>;
|
||||
};
|
||||
session.claudeSessionId = sessionId;
|
||||
session.adoptClaudeSessionId = vi.fn();
|
||||
writeTranscript(sessionId, [
|
||||
userEntry('start the job'),
|
||||
assistantEntry('Working on it.', '2026-07-21T00:00:01Z'),
|
||||
queuedEntry('actually use PowerShell', '2026-07-21T00:00:02Z'),
|
||||
queuedEntry('background agent finished', '2026-07-21T00:00:03Z', 'task-notification'),
|
||||
// The most common attachment subtype; it carries no prompt/origin at all.
|
||||
{ type: 'attachment', attachment: { type: 'total_tokens_reminder', tokens: 1 } },
|
||||
assistantEntry('Switched to PowerShell.', '2026-07-21T00:00:04Z'),
|
||||
]);
|
||||
|
||||
const { body } = await getLastResponse(sessionId, true);
|
||||
const messages = body.data.messages as Array<{ role: string; text: string; turn: number; queued?: boolean }>;
|
||||
expect(messages.map((message) => [message.role, message.text, message.turn])).toEqual([
|
||||
['user', 'start the job', 1],
|
||||
['assistant', 'Working on it.', 1],
|
||||
['user', 'actually use PowerShell', 2],
|
||||
['assistant', 'Switched to PowerShell.', 2],
|
||||
]);
|
||||
expect(messages[2].queued).toBe(true);
|
||||
expect(messages[0].queued).toBeUndefined();
|
||||
});
|
||||
|
||||
/**
|
||||
* Mostly forward insurance. A queued prompt re-emitted as a `user` row AFTER
|
||||
* its attachment row — the shape that would double-render — is not observed on
|
||||
* CLI 2.1.220-2.1.251 (0 of 163 measured 2026-09-01). The only exact-text
|
||||
* collisions are three occurrences of the same one-character nudge in a single
|
||||
* transcript, and the guard fires on one of them, which is why 163 human
|
||||
* queued rows yield 162 cards. The guard exists so a CLI that starts writing
|
||||
* both rows does not double every absorbed prompt.
|
||||
*/
|
||||
it('renders an absorbed prompt once when the CLI also writes it as a user row', async () => {
|
||||
const sessionId = harness.ctx._session.id;
|
||||
const session = harness.ctx._session as typeof harness.ctx._session & {
|
||||
claudeSessionId: string;
|
||||
adoptClaudeSessionId: ReturnType<typeof vi.fn>;
|
||||
};
|
||||
session.claudeSessionId = sessionId;
|
||||
session.adoptClaudeSessionId = vi.fn();
|
||||
writeTranscript(sessionId, [
|
||||
userEntry('go'),
|
||||
assistantEntry('OK.', '2026-07-21T00:00:01Z'),
|
||||
queuedEntry('switch to PowerShell', '2026-07-21T00:00:02Z'),
|
||||
userEntry('switch to PowerShell'),
|
||||
assistantEntry('Done.', '2026-07-21T00:00:03Z'),
|
||||
]);
|
||||
|
||||
const { body } = await getLastResponse(sessionId, true);
|
||||
const messages = body.data.messages as Array<{ role: string; text: string; queued?: boolean }>;
|
||||
const absorbed = messages.filter((message) => message.role === 'user' && message.text === 'switch to PowerShell');
|
||||
expect(absorbed).toHaveLength(1);
|
||||
expect(absorbed[0].queued).toBe(true);
|
||||
});
|
||||
|
||||
/**
|
||||
* The brief response is what agent pollers hash (skills/codeman/preamble.sh
|
||||
* last_text()). It must stay the last assistant row and must NEVER be derived
|
||||
* from messages.at(-1), which can be the user's own queued prompt.
|
||||
*/
|
||||
it('keeps the brief response on the last assistant row while a turn is in flight', async () => {
|
||||
const sessionId = harness.ctx._session.id;
|
||||
const session = harness.ctx._session as typeof harness.ctx._session & {
|
||||
claudeSessionId: string;
|
||||
adoptClaudeSessionId: ReturnType<typeof vi.fn>;
|
||||
};
|
||||
session.claudeSessionId = sessionId;
|
||||
session.adoptClaudeSessionId = vi.fn();
|
||||
writeTranscript(sessionId, [
|
||||
userEntry('go'),
|
||||
assistantEntry('Let me look.', '2026-07-21T00:00:01Z'),
|
||||
{ type: 'assistant', message: { content: [{ type: 'tool_use', id: 'x' }] } },
|
||||
{ type: 'user', message: { content: [{ type: 'tool_result', tool_use_id: 'x' }] } },
|
||||
]);
|
||||
|
||||
const brief = await getLastResponse(sessionId);
|
||||
expect(brief.body.data).toEqual({ text: 'Let me look.', timestamp: '2026-07-21T00:00:01Z' });
|
||||
|
||||
const { body } = await getLastResponse(sessionId, true);
|
||||
const messages = body.data.messages as Array<{ role: string; text: string }>;
|
||||
expect(messages.at(-1)).toMatchObject({ role: 'assistant', text: 'Let me look.' });
|
||||
expect(body.data.text).toBe('Let me look.');
|
||||
});
|
||||
|
||||
/**
|
||||
* A multi-line paste absorbed mid-turn arrives as N queued rows within a few
|
||||
* hundred milliseconds (observed: 5 rows inside ~360ms). They are one turn, so
|
||||
* the viewer renders them under one badge instead of N.
|
||||
*/
|
||||
it('groups a burst of queued prompts into one turn', async () => {
|
||||
const sessionId = harness.ctx._session.id;
|
||||
const session = harness.ctx._session as typeof harness.ctx._session & {
|
||||
claudeSessionId: string;
|
||||
adoptClaudeSessionId: ReturnType<typeof vi.fn>;
|
||||
};
|
||||
session.claudeSessionId = sessionId;
|
||||
session.adoptClaudeSessionId = vi.fn();
|
||||
writeTranscript(sessionId, [
|
||||
userEntry('go'),
|
||||
assistantEntry('OK.', '2026-07-21T00:00:01Z'),
|
||||
queuedEntry('one more thing', '2026-07-21T00:00:02.100Z'),
|
||||
queuedEntry('and the requirements are', '2026-07-21T00:00:02.360Z'),
|
||||
queuedEntry('finally, keep it fast', '2026-07-21T00:00:02.480Z'),
|
||||
assistantEntry('Understood.', '2026-07-21T00:00:05Z'),
|
||||
]);
|
||||
|
||||
const { body } = await getLastResponse(sessionId, true);
|
||||
const messages = body.data.messages as Array<{ role: string; turn: number }>;
|
||||
expect(messages.map((message) => [message.role, message.turn])).toEqual([
|
||||
['user', 1],
|
||||
['assistant', 1],
|
||||
['user', 2],
|
||||
['user', 2],
|
||||
['user', 2],
|
||||
['assistant', 2],
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
@@ -232,16 +426,18 @@ describe('GET /api/sessions/:id/last-response (claude conversation pinning)', ()
|
||||
}
|
||||
|
||||
/** Replaces the pre-seeded mock session with a Claude pane in WORKDIR. */
|
||||
function addPane(id: string, conversationId: string, lastSubmitAt: number) {
|
||||
function addPane(id: string, conversationId: string, lastSubmitAt: number, firstHand = false) {
|
||||
const base = harness.ctx._session;
|
||||
const pane = Object.create(Object.getPrototypeOf(base)) as typeof base & {
|
||||
claudeSessionId: string;
|
||||
lastSubmitAt: number;
|
||||
claudeSessionIdIsFirstHand: boolean;
|
||||
adoptClaudeSessionId: ReturnType<typeof vi.fn>;
|
||||
};
|
||||
Object.assign(pane, base, { id, mode: 'claude', workingDir: WORKDIR, docker: undefined });
|
||||
pane.claudeSessionId = conversationId;
|
||||
pane.lastSubmitAt = lastSubmitAt;
|
||||
pane.claudeSessionIdIsFirstHand = firstHand;
|
||||
pane.adoptClaudeSessionId = vi.fn((newId: string) => {
|
||||
pane.claudeSessionId = newId;
|
||||
});
|
||||
@@ -286,6 +482,41 @@ describe('GET /api/sessions/:id/last-response (claude conversation pinning)', ()
|
||||
expect(pane.adoptClaudeSessionId).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
/**
|
||||
* The whole point of the UserPromptSubmit hook: a pane driven by attaching to
|
||||
* tmux directly never bumps `lastSubmitAt` (only Codeman's own write path
|
||||
* does), so before this the correlation could not run at all for it and the
|
||||
* viewer stayed pinned to the launch conversation for the pane's whole life.
|
||||
*/
|
||||
it("trusts the pane's own hook over any history correlation", async () => {
|
||||
const pane = addPane('pane-1', 'hook-conversation', 0, true);
|
||||
writeTranscript('hook-conversation', 'the answer this pane gave', NOW - 60_000);
|
||||
// A newer, closer entry that the correlation would otherwise have claimed.
|
||||
writeTranscript('decoy-conversation', 'a stranger answer', NOW);
|
||||
writeHistory([{ sessionId: 'decoy-conversation', timestamp: NOW }]);
|
||||
|
||||
expect(await getLastResponse('pane-1')).toEqual({
|
||||
text: 'the answer this pane gave',
|
||||
timestamp: expect.any(String),
|
||||
});
|
||||
expect(pane.adoptClaudeSessionId).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('never lets a correlation override a first-hand id, even a well-anchored one', async () => {
|
||||
// Same shape as the /clear-following test above, which DOES adopt — the only
|
||||
// difference is that this pane's id came from its own hook.
|
||||
const pane = addPane('pane-1', 'before-clear', NOW, true);
|
||||
writeTranscript('before-clear', 'answer before clear', NOW - 60_000);
|
||||
writeTranscript('after-clear', 'answer after clear', NOW + 500);
|
||||
writeHistory([{ sessionId: 'after-clear', timestamp: NOW + 120 }]);
|
||||
|
||||
expect(await getLastResponse('pane-1')).toEqual({
|
||||
text: 'answer before clear',
|
||||
timestamp: expect.any(String),
|
||||
});
|
||||
expect(pane.adoptClaudeSessionId).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('credits a shared-cwd entry to the pane whose Enter is closest to it', async () => {
|
||||
const near = addPane('pane-near', 'near-conversation', NOW);
|
||||
const far = addPane('pane-far', 'far-conversation', NOW - 4_000);
|
||||
|
||||
@@ -0,0 +1,120 @@
|
||||
/**
|
||||
* @fileoverview Session.claudeSessionChain — the record of which Claude
|
||||
* conversations a pane has actually been on.
|
||||
*
|
||||
* Which conversation the response viewer reads is `Session.claudeSessionId`,
|
||||
* and `start()` reassigns it to the launch id at THREE separate points. That is
|
||||
* correct for a fresh pane and a lie for a re-attached one: a mux session that
|
||||
* survived a Codeman restart never stopped, so the CLI may have `/clear`ed hours
|
||||
* ago and moved to a conversation the launch id knows nothing about. The chain
|
||||
* is what carries that across the restart, and its tail must therefore outrank
|
||||
* the launch id on the restored path only.
|
||||
*
|
||||
* Two properties are pinned here because both were broken in ways nothing else
|
||||
* caught:
|
||||
*
|
||||
* 1. **Only a first-hand adoption extends the chain.** The id has to come from
|
||||
* the CLI's own hook payload, delivered under the pane's `$CODEMAN_SESSION_ID`.
|
||||
* A history-correlated guess writing into this record would make the
|
||||
* "showed a stranger's conversation" bug permanent instead of transient.
|
||||
* 2. **A restored conversation survives every reset point.** The mux branch and
|
||||
* the unconditional "third reset point" after it both reassign the field, so
|
||||
* patching only the first leaves the restore silently undone.
|
||||
*
|
||||
* Port: N/A
|
||||
*/
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { resolve } from 'node:path';
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { Session } from '../src/session.js';
|
||||
|
||||
describe('Session claude conversation chain', () => {
|
||||
it('extends the chain only for a first-hand adoption', () => {
|
||||
const session = new Session({ workingDir: '/tmp', mode: 'claude' });
|
||||
|
||||
// A correlated guess: adopted for display, but never recorded.
|
||||
session.adoptClaudeSessionId('guessed-conversation');
|
||||
expect(session.claudeSessionId).toBe('guessed-conversation');
|
||||
expect(session.claudeSessionChain).toEqual([]);
|
||||
expect(session.claudeSessionIdIsFirstHand).toBe(false);
|
||||
|
||||
// The CLI's own hook: recorded.
|
||||
session.adoptClaudeSessionId('hook-conversation', { firstHand: true });
|
||||
expect(session.claudeSessionChain).toEqual(['hook-conversation']);
|
||||
expect(session.claudeSessionIdIsFirstHand).toBe(true);
|
||||
});
|
||||
|
||||
it('records a /clear successor once, however many prompts report it', () => {
|
||||
const session = new Session({ workingDir: '/tmp', mode: 'claude' });
|
||||
|
||||
session.adoptClaudeSessionId('conv-1', { firstHand: true });
|
||||
session.adoptClaudeSessionId('conv-1', { firstHand: true }); // every prompt reports the same id
|
||||
session.adoptClaudeSessionId('conv-2', { firstHand: true }); // the user ran /clear
|
||||
|
||||
expect(session.claudeSessionChain).toEqual(['conv-1', 'conv-2']);
|
||||
expect(session.claudeSessionId).toBe('conv-2');
|
||||
});
|
||||
|
||||
it('moves a resumed conversation to the tail instead of duplicating it', () => {
|
||||
const session = new Session({ workingDir: '/tmp', mode: 'claude' });
|
||||
|
||||
session.adoptClaudeSessionId('conv-1', { firstHand: true });
|
||||
session.adoptClaudeSessionId('conv-2', { firstHand: true });
|
||||
session.adoptClaudeSessionId('conv-1', { firstHand: true }); // /resume back
|
||||
|
||||
expect(session.claudeSessionChain).toEqual(['conv-2', 'conv-1']);
|
||||
});
|
||||
|
||||
it('round-trips the chain through toState and re-pins the conversation on restore', () => {
|
||||
const original = new Session({ workingDir: '/tmp', mode: 'claude' });
|
||||
original.adoptClaudeSessionId('conv-1', { firstHand: true });
|
||||
original.adoptClaudeSessionId('conv-2', { firstHand: true });
|
||||
|
||||
const state = original.toState() as { claudeSessionChain?: string[] };
|
||||
expect(state.claudeSessionChain).toEqual(['conv-1', 'conv-2']);
|
||||
|
||||
// Boot recovery rebuilds the pane from that state. The launch id would point
|
||||
// the viewer at the pre-/clear conversation; the chain's tail corrects it.
|
||||
const restored = new Session({
|
||||
workingDir: '/tmp',
|
||||
mode: 'claude',
|
||||
id: original.id,
|
||||
claudeSessionChain: state.claudeSessionChain,
|
||||
});
|
||||
expect(restored.claudeSessionId).toBe('conv-2');
|
||||
// ⚠️ NOT restored: a persisted claim is not a fact. The pane re-earns the
|
||||
// guess-free path from its next hook.
|
||||
expect(restored.claudeSessionIdIsFirstHand).toBe(false);
|
||||
});
|
||||
|
||||
it('omits the chain from toState when the pane never moved conversation', () => {
|
||||
const session = new Session({ workingDir: '/tmp', mode: 'claude' });
|
||||
expect((session.toState() as { claudeSessionChain?: string[] }).claudeSessionChain).toBeUndefined();
|
||||
});
|
||||
|
||||
it('applies the restored conversation at EVERY reset point in start()', () => {
|
||||
// ⚠️ Structural pin, not a behavioural one: exercising start() needs a real
|
||||
// PTY and mux. start() reassigns _claudeSessionId at three points, and the
|
||||
// last one runs unconditionally AFTER the mux branch — so patching only the
|
||||
// mux branch leaves the restore silently undone, which is what shipped
|
||||
// before this existed. Every assignment built from the launch-id fallback
|
||||
// must therefore carry `restoredConversation` first.
|
||||
const source = readFileSync(resolve(import.meta.dirname, '../src/session.ts'), 'utf8');
|
||||
// The tail of the chain grows as each CLI gains a resume alias of its own
|
||||
// (omp, then codex), so the pattern pins the two ends and lets the middle
|
||||
// widen. A `[^;]` run cannot cross a statement boundary, so each match is
|
||||
// still one assignment.
|
||||
const fallbackAssignments = source.match(/_claudeSessionId =[^;]*?_resumeSessionId[^;]*?this\.id;/g);
|
||||
expect(fallbackAssignments).not.toBeNull();
|
||||
expect(fallbackAssignments!.length).toBeGreaterThanOrEqual(2);
|
||||
for (const assignment of fallbackAssignments!) {
|
||||
expect(assignment).toContain('restoredConversation ||');
|
||||
}
|
||||
});
|
||||
|
||||
it('leaves a fresh pane on its launch id', () => {
|
||||
const session = new Session({ workingDir: '/tmp', mode: 'claude' });
|
||||
expect(session.claudeSessionId).toBe(session.id);
|
||||
expect(session.claudeSessionChain).toEqual([]);
|
||||
});
|
||||
});
|
||||
@@ -41,6 +41,10 @@ delete process.env.CODEMAN_USERNAME;
|
||||
// __codemanGestureAvailable flag), breaking byte-identity assertions
|
||||
// (test/server-index-title.test.ts) when the shell exports CODEMAN_GESTURE=1.
|
||||
delete process.env.CODEMAN_GESTURE;
|
||||
// CODEMAN_BASE_URL (#381) is read by the WebServer constructor as a fallback; an
|
||||
// operator who exports it (exactly who the feature is for) would otherwise see the
|
||||
// root-install byte-identity assertions fail.
|
||||
delete process.env.CODEMAN_BASE_URL;
|
||||
|
||||
// Instance selection is PROCESS-WIDE and is what `src/config/instance.ts` derives
|
||||
// both the data dir and the tmux socket from, so a shell that exports any of these
|
||||
|
||||
@@ -30,6 +30,7 @@ const STRIPPED_ENV_VARS: Array<[name: string, why: string]> = [
|
||||
['CODEMAN_PASSWORD', 'auth from a running instance would make protected routes behave differently'],
|
||||
['CODEMAN_USERNAME', 'same, and it changes which owner scoping resolves to'],
|
||||
['CODEMAN_GESTURE', 'flips renderIndexHtml output and breaks byte-identity assertions'],
|
||||
['CODEMAN_BASE_URL', 'mounts the server under a sub-path and breaks the root-install byte-identity assertions'],
|
||||
['CODEMAN_INSTANCE', 'moves the data dir to ~/.codeman-<name> and the tmux socket to codeman-<name>'],
|
||||
['CODEMAN_DATA_DIR', 'ABSOLUTE override: bypasses the temp HOME and points the suite at a real data dir'],
|
||||
['CODEMAN_TMUX_SOCKET', 'renames the socket resolveTmuxSocketName() returns'],
|
||||
|
||||
@@ -684,3 +684,39 @@ describe('referrer policy on proxied responses', () => {
|
||||
expect(headers['referrer-policy']).toBe('same-origin');
|
||||
});
|
||||
});
|
||||
|
||||
describe('reverse-proxy base path', () => {
|
||||
const BASE = '/codeman';
|
||||
const BASED_PREFIX = `${BASE}/webview/${CAP}/`;
|
||||
|
||||
it('rides the mount into the iframe prefix', () => {
|
||||
expect(proxyPrefixFor(CAP, BASE)).toBe(BASED_PREFIX);
|
||||
expect(proxyPrefixFor(CAP, '')).toBe(PREFIX); // root unchanged
|
||||
});
|
||||
|
||||
it('rewrites HTML (base tag, root-absolute attrs, shim) under the mount', () => {
|
||||
const out = rewriteHtml('<html><head></head><body><img src="/logo.png"></body></html>', CAP, BASE);
|
||||
expect(out).toContain(`<base href="${BASED_PREFIX}">`);
|
||||
expect(out).toContain(`src="${BASED_PREFIX}logo.png"`);
|
||||
// The runtime shim's rewrite target is the base-prefixed path.
|
||||
expect(out).toContain(JSON.stringify(BASED_PREFIX));
|
||||
});
|
||||
|
||||
it('rebases Set-Cookie Path onto the mounted prefix so the browser sends it back', () => {
|
||||
expect(rewriteSetCookie('sid=abc; Path=/', CAP, true, BASE)).toContain(`Path=${BASED_PREFIX}`);
|
||||
expect(rewriteSetCookie('sid=abc; HttpOnly', CAP, true, BASE)).toContain(`Path=${BASED_PREFIX}`);
|
||||
});
|
||||
|
||||
it('rewrites a same-origin Location into the mounted prefix', () => {
|
||||
const requestUrl = new URL('http://127.0.0.1:4000/app');
|
||||
expect(rewriteLocation('/dashboard?x=1', requestUrl, CAP, BASE)).toBe(`${BASED_PREFIX}dashboard?x=1`);
|
||||
});
|
||||
|
||||
it('extracts the capability from a browser Referer that carries the mount prefix', () => {
|
||||
expect(capabilityFromReferer(`https://box.ts.net${BASED_PREFIX}page`, BASE)).toBe(CAP);
|
||||
// A same-named sibling path must not be mistaken for the mount.
|
||||
expect(capabilityFromReferer(`https://box.ts.net/codeman-docs/webview/${CAP}/page`, BASE)).toBeNull();
|
||||
// Without the base arg the prefixed Referer no longer matches (documents why the arg exists).
|
||||
expect(capabilityFromReferer(`https://box.ts.net${BASED_PREFIX}page`)).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user