mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-07 16:09:43 +02:00
fix: address PR review findings for Gemini run mode + Ralph todo-config
Gemini (PR #134) blockers: - runGemini() now unwraps the {success,data} envelope: status check reads .data.available, quick-start reads data.data.sessionId (was reading the raw shape, so the Run-Gemini button could never start a session). - setGeminiEnvVars() now uses the socket-scoped ${this.tmux()} setenv instead of bare tmux — Gemini/Google auth env vars were targeting the wrong tmux server and silently failing on every install. Gemini parity polish: - gemini tab-mode badge ('gm') + .tab-mode.gemini CSS; kill-dialog label 'Kill Tmux & Gemini'; codeman doctor dependency-registry entry; export isGeminiAvailable from utils barrel; COLORTERM=truecolor + unset NO_COLOR; add gemini to isAltScreenStripMode (Ink TUI, repaints inline like Codex/Claude). - Revert 4 system-routes.test.ts envelope assertions weakened to (body.message ?? body.error) back to (body.success === false). - Add a runGemini() vm-sandbox test that drives the envelope path end-to-end. Ralph todo-config (PR #135): maxTodos/todoExpirationMinutes are now persisted and read back — surfaced via the loopState getter (RalphTrackerState) into toState()/SSE broadcast and restored in restoreState(), mirroring maxIterations. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -90,6 +90,14 @@ export const DEPENDENCY_REGISTRY: ToolDependency[] = [
|
|||||||
usedBy: ['Codex sessions'],
|
usedBy: ['Codex sessions'],
|
||||||
resolvers: [{ match: ALL, resolver: { kind: 'path', bins: ['codex'], versionArg: '--version' } }],
|
resolvers: [{ match: ALL, resolver: { kind: 'path', bins: ['codex'], versionArg: '--version' } }],
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
id: 'gemini',
|
||||||
|
label: 'Gemini CLI',
|
||||||
|
category: 'core',
|
||||||
|
required: false,
|
||||||
|
usedBy: ['Gemini sessions'],
|
||||||
|
resolvers: [{ match: ALL, resolver: { kind: 'path', bins: ['gemini'], versionArg: '--version' } }],
|
||||||
|
},
|
||||||
{
|
{
|
||||||
id: 'libreoffice',
|
id: 'libreoffice',
|
||||||
label: 'LibreOffice',
|
label: 'LibreOffice',
|
||||||
|
|||||||
@@ -1059,6 +1059,10 @@ export class RalphTracker extends EventEmitter {
|
|||||||
planVersion: this.planTracker.planVersion,
|
planVersion: this.planTracker.planVersion,
|
||||||
planHistoryLength: this.planTracker.getPlanHistory().length,
|
planHistoryLength: this.planTracker.getPlanHistory().length,
|
||||||
completionConfidence: this._lastCompletionConfidence,
|
completionConfidence: this._lastCompletionConfidence,
|
||||||
|
// Surface the live todo-config so it persists (toState) and reads back into
|
||||||
|
// the Session Options modal (broadcast) — mirrors maxIterations round-trip.
|
||||||
|
maxTodos: this._maxTodos,
|
||||||
|
todoExpirationMinutes: this.todoExpirationMinutes,
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -2345,6 +2349,12 @@ export class RalphTracker extends EventEmitter {
|
|||||||
...loopState,
|
...loopState,
|
||||||
enabled: loopState.enabled ?? false,
|
enabled: loopState.enabled ?? false,
|
||||||
};
|
};
|
||||||
|
// Restore the per-session todo-config into the live fields used by the hot
|
||||||
|
// paths (eviction cap + expiry). Setters ignore non-positive values.
|
||||||
|
if (typeof loopState.maxTodos === 'number') this.setMaxTodos(loopState.maxTodos);
|
||||||
|
if (typeof loopState.todoExpirationMinutes === 'number') {
|
||||||
|
this.setTodoExpirationMinutes(loopState.todoExpirationMinutes);
|
||||||
|
}
|
||||||
this._todos.clear();
|
this._todos.clear();
|
||||||
for (const todo of todos) {
|
for (const todo of todos) {
|
||||||
this._todos.set(todo.id, {
|
this._todos.set(todo.id, {
|
||||||
|
|||||||
+7
-6
@@ -157,14 +157,15 @@ function getModeLabel(mode: SessionMode): string {
|
|||||||
* that we strip so the browser keeps everything in the main buffer with scrollback
|
* that we strip so the browser keeps everything in the main buffer with scrollback
|
||||||
* reachable (the strip runs on both the live stream and the buffer replay).
|
* reachable (the strip runs on both the live stream and the buffer replay).
|
||||||
*
|
*
|
||||||
* Codex and Claude Code are known, controlled TUIs that repaint via cursor
|
* Codex, Claude Code, and Gemini are known, controlled (Ink/React) TUIs that
|
||||||
* positioning, so dropping the alt-screen switch is safe — content stays in the
|
* repaint via cursor positioning, so dropping the alt-screen switch is safe —
|
||||||
* normal buffer. Excluded: `shell` (arbitrary programs like vim/less/htop
|
* content stays in the normal buffer. Excluded: `shell` (arbitrary programs like
|
||||||
* legitimately need the alt screen) and `opencode` (renders its own TUI that
|
* vim/less/htop legitimately need the alt screen) and `opencode` (renders its own
|
||||||
* may rely on it). Keep parity with the replay-side strip in session-routes.ts.
|
* TUI that may rely on it). Keep parity with the replay-side strip in
|
||||||
|
* session-routes.ts.
|
||||||
*/
|
*/
|
||||||
export function isAltScreenStripMode(mode: SessionMode): boolean {
|
export function isAltScreenStripMode(mode: SessionMode): boolean {
|
||||||
return mode === 'codex' || mode === 'claude';
|
return mode === 'codex' || mode === 'claude' || mode === 'gemini';
|
||||||
}
|
}
|
||||||
|
|
||||||
// Note: Claude CLI PATH resolution moved to session-cli-builder.ts (buildClaudeEnv)
|
// Note: Claude CLI PATH resolution moved to session-cli-builder.ts (buildClaudeEnv)
|
||||||
|
|||||||
+5
-5
@@ -720,7 +720,7 @@ function setCodexEnvVars(tmuxCmd: string, muxName: string): void {
|
|||||||
* Gemini Pro/Ultra users usually authenticate via cached Google login; these
|
* Gemini Pro/Ultra users usually authenticate via cached Google login; these
|
||||||
* variables cover API-key and Vertex AI paths without putting secrets in ps.
|
* variables cover API-key and Vertex AI paths without putting secrets in ps.
|
||||||
*/
|
*/
|
||||||
function setGeminiEnvVars(muxName: string): void {
|
function setGeminiEnvVars(tmuxCmd: string, muxName: string): void {
|
||||||
const sensitiveVars = [
|
const sensitiveVars = [
|
||||||
'GEMINI_API_KEY',
|
'GEMINI_API_KEY',
|
||||||
'GEMINI_MODEL',
|
'GEMINI_MODEL',
|
||||||
@@ -735,7 +735,7 @@ function setGeminiEnvVars(muxName: string): void {
|
|||||||
if (val) {
|
if (val) {
|
||||||
const escaped = val.replace(/'/g, "'\\''");
|
const escaped = val.replace(/'/g, "'\\''");
|
||||||
try {
|
try {
|
||||||
execSync(`tmux setenv -t '${muxName}' ${key} '${escaped}'`, {
|
execSync(`${tmuxCmd} setenv -t '${muxName}' ${key} '${escaped}'`, {
|
||||||
encoding: 'utf8',
|
encoding: 'utf8',
|
||||||
timeout: EXEC_TIMEOUT_MS,
|
timeout: EXEC_TIMEOUT_MS,
|
||||||
stdio: ['pipe', 'pipe', 'pipe'],
|
stdio: ['pipe', 'pipe', 'pipe'],
|
||||||
@@ -929,8 +929,8 @@ export class TmuxManager extends EventEmitter implements TerminalMultiplexer {
|
|||||||
const exports = [
|
const exports = [
|
||||||
'export LANG=en_US.UTF-8',
|
'export LANG=en_US.UTF-8',
|
||||||
'export LC_ALL=en_US.UTF-8',
|
'export LC_ALL=en_US.UTF-8',
|
||||||
mode === 'codex' ? 'export COLORTERM=truecolor' : 'unset COLORTERM',
|
mode === 'codex' || mode === 'gemini' ? 'export COLORTERM=truecolor' : 'unset COLORTERM',
|
||||||
...(mode === 'codex' ? ['unset NO_COLOR'] : []),
|
...(mode === 'codex' || mode === 'gemini' ? ['unset NO_COLOR'] : []),
|
||||||
'export CODEMAN_MUX=1',
|
'export CODEMAN_MUX=1',
|
||||||
`export CODEMAN_SESSION_ID=${sessionId}`,
|
`export CODEMAN_SESSION_ID=${sessionId}`,
|
||||||
`export CODEMAN_MUX_NAME=${muxName}`,
|
`export CODEMAN_MUX_NAME=${muxName}`,
|
||||||
@@ -1034,7 +1034,7 @@ export class TmuxManager extends EventEmitter implements TerminalMultiplexer {
|
|||||||
* Configure Gemini-specific environment on a tmux session.
|
* Configure Gemini-specific environment on a tmux session.
|
||||||
*/
|
*/
|
||||||
private _configureGemini(muxName: string): void {
|
private _configureGemini(muxName: string): void {
|
||||||
setGeminiEnvVars(muxName);
|
setGeminiEnvVars(this.tmux(), muxName);
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@@ -90,6 +90,10 @@ export interface RalphTrackerState {
|
|||||||
cycleCount: number;
|
cycleCount: number;
|
||||||
/** Maximum iterations if detected */
|
/** Maximum iterations if detected */
|
||||||
maxIterations: number | null;
|
maxIterations: number | null;
|
||||||
|
/** Max todos retained for this session before FIFO eviction (persisted; default = global cap) */
|
||||||
|
maxTodos?: number;
|
||||||
|
/** Todo auto-expiry in minutes (persisted; default = global TODO_EXPIRY_MS) */
|
||||||
|
todoExpirationMinutes?: number;
|
||||||
/** Timestamp of last activity */
|
/** Timestamp of last activity */
|
||||||
lastActivity: number;
|
lastActivity: number;
|
||||||
/** Elapsed hours if detected */
|
/** Elapsed hours if detected */
|
||||||
|
|||||||
+1
-1
@@ -29,4 +29,4 @@ export { wrapWithNice } from './nice-wrapper.js';
|
|||||||
export { findClaudeDir, getAugmentedPath } from './claude-cli-resolver.js';
|
export { findClaudeDir, getAugmentedPath } from './claude-cli-resolver.js';
|
||||||
export { resolveOpenCodeDir } from './opencode-cli-resolver.js';
|
export { resolveOpenCodeDir } from './opencode-cli-resolver.js';
|
||||||
export { resolveCodexDir, isCodexAvailable } from './codex-cli-resolver.js';
|
export { resolveCodexDir, isCodexAvailable } from './codex-cli-resolver.js';
|
||||||
export { resolveGeminiDir } from './gemini-cli-resolver.js';
|
export { resolveGeminiDir, isGeminiAvailable } from './gemini-cli-resolver.js';
|
||||||
|
|||||||
@@ -3004,7 +3004,7 @@ class CodemanApp {
|
|||||||
<span class="tab-status ${status}" aria-hidden="true"></span>
|
<span class="tab-status ${status}" aria-hidden="true"></span>
|
||||||
<span class="tab-info">
|
<span class="tab-info">
|
||||||
<span class="tab-name-row">
|
<span class="tab-name-row">
|
||||||
${mode === 'shell' ? '<span class="tab-mode shell" aria-hidden="true">sh</span>' : mode === 'opencode' ? '<span class="tab-mode opencode" aria-hidden="true">oc</span>' : mode === 'codex' ? '<span class="tab-mode codex" aria-hidden="true">cx</span>' : ''}
|
${mode === 'shell' ? '<span class="tab-mode shell" aria-hidden="true">sh</span>' : mode === 'opencode' ? '<span class="tab-mode opencode" aria-hidden="true">oc</span>' : mode === 'codex' ? '<span class="tab-mode codex" aria-hidden="true">cx</span>' : mode === 'gemini' ? '<span class="tab-mode gemini" aria-hidden="true">gm</span>' : ''}
|
||||||
<span class="tab-name" data-session-id="${id}">${(() => { const p = parseSessionPrefix(name); return p && p.suffix ? '<span class="tab-prefix">' + escapeHtml(p.prefix) + '</span><span class="tab-suffix">: ' + escapeHtml(p.suffix) + '</span>' : escapeHtml(name); })()}</span>
|
<span class="tab-name" data-session-id="${id}">${(() => { const p = parseSessionPrefix(name); return p && p.suffix ? '<span class="tab-prefix">' + escapeHtml(p.prefix) + '</span><span class="tab-suffix">: ' + escapeHtml(p.suffix) + '</span>' : escapeHtml(name); })()}</span>
|
||||||
<span class="tab-detached-badge" aria-hidden="true">detached</span>
|
<span class="tab-detached-badge" aria-hidden="true">detached</span>
|
||||||
</span>
|
</span>
|
||||||
@@ -3988,7 +3988,9 @@ class CodemanApp {
|
|||||||
? 'Kill Tmux & OpenCode'
|
? 'Kill Tmux & OpenCode'
|
||||||
: session.mode === 'codex'
|
: session.mode === 'codex'
|
||||||
? 'Kill Tmux & Codex'
|
? 'Kill Tmux & Codex'
|
||||||
: 'Kill Tmux & Claude Code';
|
: session.mode === 'gemini'
|
||||||
|
? 'Kill Tmux & Gemini'
|
||||||
|
: 'Kill Tmux & Claude Code';
|
||||||
}
|
}
|
||||||
|
|
||||||
document.getElementById('closeConfirmModal').classList.add('active');
|
document.getElementById('closeConfirmModal').classList.add('active');
|
||||||
|
|||||||
@@ -653,7 +653,7 @@ Object.assign(CodemanApp.prototype, {
|
|||||||
|
|
||||||
try {
|
try {
|
||||||
const statusRes = await fetch('/api/gemini/status');
|
const statusRes = await fetch('/api/gemini/status');
|
||||||
const status = await statusRes.json();
|
const status = (await statusRes.json()).data;
|
||||||
if (!status.available) {
|
if (!status.available) {
|
||||||
this.terminal.writeln('\x1b[1;31m Gemini CLI not found.\x1b[0m');
|
this.terminal.writeln('\x1b[1;31m Gemini CLI not found.\x1b[0m');
|
||||||
this.terminal.writeln('\x1b[90m Install with: npm install -g @google/gemini-cli\x1b[0m');
|
this.terminal.writeln('\x1b[90m Install with: npm install -g @google/gemini-cli\x1b[0m');
|
||||||
@@ -674,8 +674,8 @@ Object.assign(CodemanApp.prototype, {
|
|||||||
const data = await res.json();
|
const data = await res.json();
|
||||||
if (!data.success) throw new Error(data.error || 'Failed to start Gemini');
|
if (!data.success) throw new Error(data.error || 'Failed to start Gemini');
|
||||||
|
|
||||||
if (data.sessionId) {
|
if (data.data.sessionId) {
|
||||||
await this.selectSession(data.sessionId);
|
await this.selectSession(data.data.sessionId);
|
||||||
}
|
}
|
||||||
|
|
||||||
this.terminal.focus();
|
this.terminal.focus();
|
||||||
@@ -796,6 +796,8 @@ Object.assign(CodemanApp.prototype, {
|
|||||||
enabled: ralphState?.loop?.enabled ?? session.ralphLoop?.enabled ?? false,
|
enabled: ralphState?.loop?.enabled ?? session.ralphLoop?.enabled ?? false,
|
||||||
completionPhrase: ralphState?.loop?.completionPhrase || session.ralphLoop?.completionPhrase || '',
|
completionPhrase: ralphState?.loop?.completionPhrase || session.ralphLoop?.completionPhrase || '',
|
||||||
maxIterations: ralphState?.loop?.maxIterations || session.ralphLoop?.maxIterations || 0,
|
maxIterations: ralphState?.loop?.maxIterations || session.ralphLoop?.maxIterations || 0,
|
||||||
|
maxTodos: ralphState?.loop?.maxTodos || session.ralphLoop?.maxTodos,
|
||||||
|
todoExpirationMinutes: ralphState?.loop?.todoExpirationMinutes || session.ralphLoop?.todoExpirationMinutes,
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -1178,6 +1178,11 @@ body.solo-mode .btn-lifecycle-log {
|
|||||||
color: #a855f7;
|
color: #a855f7;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
.session-tab .tab-mode.gemini {
|
||||||
|
background: rgba(138, 180, 248, 0.2);
|
||||||
|
color: #8ab4f8;
|
||||||
|
}
|
||||||
|
|
||||||
/* Timer Banner - Compact */
|
/* Timer Banner - Compact */
|
||||||
.timer-banner {
|
.timer-banner {
|
||||||
display: flex;
|
display: flex;
|
||||||
|
|||||||
@@ -177,7 +177,7 @@ describe('system-routes', () => {
|
|||||||
});
|
});
|
||||||
expect(res.statusCode).toBe(400);
|
expect(res.statusCode).toBe(400);
|
||||||
const body = JSON.parse(res.body);
|
const body = JSON.parse(res.body);
|
||||||
expect(body.message ?? body.error).toBeTruthy();
|
expect(body.success).toBe(false);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -432,7 +432,7 @@ describe('system-routes', () => {
|
|||||||
});
|
});
|
||||||
expect(res.statusCode).toBe(400);
|
expect(res.statusCode).toBe(400);
|
||||||
const body = JSON.parse(res.body);
|
const body = JSON.parse(res.body);
|
||||||
expect(body.message ?? body.error).toBeTruthy();
|
expect(body.success).toBe(false);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('saves lastUsedCase as partial update without overwriting other settings', async () => {
|
it('saves lastUsedCase as partial update without overwriting other settings', async () => {
|
||||||
@@ -530,7 +530,7 @@ describe('system-routes', () => {
|
|||||||
});
|
});
|
||||||
expect(res.statusCode).toBe(400);
|
expect(res.statusCode).toBe(400);
|
||||||
const body = JSON.parse(res.body);
|
const body = JSON.parse(res.body);
|
||||||
expect(body.message ?? body.error).toBeTruthy();
|
expect(body.success).toBe(false);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -587,7 +587,7 @@ describe('system-routes', () => {
|
|||||||
});
|
});
|
||||||
expect(res.statusCode).toBe(400);
|
expect(res.statusCode).toBe(400);
|
||||||
const body = JSON.parse(res.body);
|
const body = JSON.parse(res.body);
|
||||||
expect(body.message ?? body.error).toBeTruthy();
|
expect(body.success).toBe(false);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -143,3 +143,56 @@ describe('Codex quick start settings', () => {
|
|||||||
expect(selected).toEqual(['sess-1']);
|
expect(selected).toEqual(['sess-1']);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('Gemini quick start', () => {
|
||||||
|
// Regression guard for the ApiResponse-envelope unwrap in runGemini(): the
|
||||||
|
// status check must read `.data.available` and the quick-start response must
|
||||||
|
// read `.data.sessionId`. Reading the raw shape (pre-fix) silently bails on
|
||||||
|
// the status check and never selects the new tab — exactly the two blockers
|
||||||
|
// caught in PR #134 review.
|
||||||
|
it('drives runGemini() through the {success,data} envelope and selects the new session', async () => {
|
||||||
|
const elements: Record<string, any> = {
|
||||||
|
quickStartCase: { value: 'gemini-case' },
|
||||||
|
};
|
||||||
|
const requests: Array<{ url: string; body?: any }> = [];
|
||||||
|
const CodemanApp = function CodemanApp(this: any) {};
|
||||||
|
|
||||||
|
const context = vm.createContext({
|
||||||
|
CodemanApp,
|
||||||
|
localStorage: { getItem: () => null, setItem: () => {} },
|
||||||
|
document: { getElementById: (id: string) => elements[id] ?? null },
|
||||||
|
// Mock responses use the real wire shape: the server.ts preSerialization
|
||||||
|
// hook wraps raw route payloads into the { success, data } envelope.
|
||||||
|
fetch: async (url: string, init?: { body?: string }) => {
|
||||||
|
requests.push({ url, body: init?.body ? JSON.parse(init.body) : undefined });
|
||||||
|
if (url === '/api/gemini/status') return { json: async () => ({ success: true, data: { available: true } }) };
|
||||||
|
if (url === '/api/quick-start')
|
||||||
|
return { json: async () => ({ success: true, data: { sessionId: 'sess-gm' } }) };
|
||||||
|
throw new Error(`unexpected fetch: ${url}`);
|
||||||
|
},
|
||||||
|
console,
|
||||||
|
});
|
||||||
|
|
||||||
|
const sessionUi = readFileSync(resolve(import.meta.dirname, '../src/web/public/session-ui.js'), 'utf8');
|
||||||
|
vm.runInContext(sessionUi, context, { filename: 'session-ui.js' });
|
||||||
|
|
||||||
|
const app = new (CodemanApp as any)();
|
||||||
|
app.terminal = { clear: () => {}, writeln: () => {}, focus: () => {} };
|
||||||
|
app.loadAppSettingsFromStorage = () => ({});
|
||||||
|
app.getCaseSettings = () => ({});
|
||||||
|
app.buildEnvOverrides = () => ({});
|
||||||
|
const selected: string[] = [];
|
||||||
|
app.selectSession = async (id: string) => {
|
||||||
|
selected.push(id);
|
||||||
|
};
|
||||||
|
|
||||||
|
await app.runGemini();
|
||||||
|
|
||||||
|
expect(requests.find((req) => req.url === '/api/quick-start')?.body).toMatchObject({
|
||||||
|
caseName: 'gemini-case',
|
||||||
|
mode: 'gemini',
|
||||||
|
geminiConfig: { approvalMode: 'yolo' },
|
||||||
|
});
|
||||||
|
expect(selected).toEqual(['sess-gm']);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user