mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-02 13:39:41 +02:00
fix: address code review findings for OpenCode integration
- Add _isStopped guard to OpenCode 3s readiness timeout (session.ts) - Block respawn for opencode sessions on interactive-respawn and respawn/enable routes (server.ts) - Fail fast in direct PTY fallback for OpenCode mode (session.ts) - Validate configContent as JSON at schema level (schemas.ts) - Update JSDoc example for createSession options API (tmux-manager.ts) - Un-hide Context tab for OpenCode sessions (index.html) - Add OpenCode UI tests (opencode-resize.test.ts) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -989,6 +989,7 @@ export class Session extends EventEmitter {
|
||||
// Emit needsRefresh so the client fetches the full buffer once the TUI has rendered.
|
||||
this._promptCheckTimeout = setTimeout(() => {
|
||||
this._promptCheckTimeout = null;
|
||||
if (this._isStopped) return;
|
||||
this._status = 'idle';
|
||||
this.emit('needsRefresh');
|
||||
}, 3000);
|
||||
@@ -1035,6 +1036,10 @@ export class Session extends EventEmitter {
|
||||
|
||||
// Fallback to direct PTY if mux is not used
|
||||
if (!this.ptyProcess) {
|
||||
// OpenCode sessions require tmux for env var injection (API keys via setenv)
|
||||
if (this.mode === 'opencode') {
|
||||
throw new Error('OpenCode sessions require tmux. Direct PTY fallback is not supported.');
|
||||
}
|
||||
try {
|
||||
// Pass --session-id to use the SAME ID as the Claudeman session
|
||||
// This ensures subagents can be directly matched to the correct tab
|
||||
|
||||
+1
-1
@@ -258,7 +258,7 @@ function setOpenCodeConfigContent(muxName: string, config?: OpenCodeConfig): voi
|
||||
* const manager = new TmuxManager();
|
||||
*
|
||||
* // Create a tmux session for Claude
|
||||
* const session = await manager.createSession(sessionId, '/project', 'claude');
|
||||
* const session = await manager.createSession({ sessionId, workingDir: '/project', mode: 'claude' });
|
||||
*
|
||||
* // Send input (single command, no delay!)
|
||||
* manager.sendInput(sessionId, '/clear\r');
|
||||
|
||||
@@ -510,7 +510,7 @@
|
||||
<!-- Modal Tabs -->
|
||||
<div class="modal-tabs">
|
||||
<button class="modal-tab-btn active" data-tab="respawn" data-claude-only onclick="app.switchOptionsTab('respawn')">Respawn</button>
|
||||
<button class="modal-tab-btn" data-tab="context" data-claude-only onclick="app.switchOptionsTab('context')">Context</button>
|
||||
<button class="modal-tab-btn" data-tab="context" onclick="app.switchOptionsTab('context')">Context</button>
|
||||
<button class="modal-tab-btn" data-tab="ralph" data-claude-only onclick="app.switchOptionsTab('ralph')">Ralph / Todo</button>
|
||||
<button class="modal-tab-btn" data-tab="summary" onclick="app.switchOptionsTab('summary')">Summary</button>
|
||||
</div>
|
||||
|
||||
+4
-1
@@ -70,7 +70,10 @@ const OpenCodeConfigSchema = z.object({
|
||||
autoAllowTools: z.boolean().optional(),
|
||||
continueSession: z.string().max(100).regex(/^[a-zA-Z0-9_-]+$/).optional(),
|
||||
forkSession: z.boolean().optional(),
|
||||
configContent: z.string().max(10000).optional(),
|
||||
configContent: z.string().max(10000).refine(
|
||||
(val) => { try { JSON.parse(val); return true; } catch { return false; } },
|
||||
{ message: 'configContent must be valid JSON' },
|
||||
).optional(),
|
||||
}).optional();
|
||||
|
||||
export const CreateSessionSchema = z.object({
|
||||
|
||||
+11
-2
@@ -2026,10 +2026,14 @@ export class WebServer extends EventEmitter {
|
||||
return createErrorResponse(ApiErrorCode.SESSION_BUSY, 'Session is busy');
|
||||
}
|
||||
|
||||
// Respawn is not supported for opencode sessions
|
||||
if (session.mode === 'opencode') {
|
||||
return createErrorResponse(ApiErrorCode.INVALID_INPUT, 'Respawn is not supported for opencode sessions');
|
||||
}
|
||||
|
||||
try {
|
||||
// Auto-detect completion phrase from CLAUDE.md BEFORE starting (only if globally enabled and not explicitly disabled by user)
|
||||
// Ralph tracker is not supported for opencode sessions
|
||||
if (session.mode !== 'opencode' && this.store.getConfig().ralphEnabled && !session.ralphTracker.autoEnableDisabled) {
|
||||
if (this.store.getConfig().ralphEnabled && !session.ralphTracker.autoEnableDisabled) {
|
||||
autoConfigureRalph(session, session.workingDir, () => {});
|
||||
if (!session.ralphTracker.enabled) {
|
||||
session.ralphTracker.enable();
|
||||
@@ -2084,6 +2088,11 @@ export class WebServer extends EventEmitter {
|
||||
return createErrorResponse(ApiErrorCode.NOT_FOUND, 'Session not found');
|
||||
}
|
||||
|
||||
// Respawn is not supported for opencode sessions
|
||||
if (session.mode === 'opencode') {
|
||||
return createErrorResponse(ApiErrorCode.INVALID_INPUT, 'Respawn is not supported for opencode sessions');
|
||||
}
|
||||
|
||||
// Check if session is running (has a PID)
|
||||
if (!session.pid) {
|
||||
return createErrorResponse(ApiErrorCode.OPERATION_FAILED, 'Session is not running. Start it first.');
|
||||
|
||||
@@ -0,0 +1,345 @@
|
||||
/**
|
||||
* OpenCode session UI tests
|
||||
*
|
||||
* Tests OpenCode-specific UI behavior:
|
||||
* - Initial terminal resize (not stuck at 120x40)
|
||||
* - Close modal shows "Kill Tmux & OpenCode" (not "Claude Code")
|
||||
* - needsRefresh handler sends resize
|
||||
*
|
||||
* Port: 3211 (opencode UI tests)
|
||||
*
|
||||
* Run: npx vitest run test/opencode-resize.test.ts
|
||||
*/
|
||||
|
||||
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
|
||||
import { chromium, type Browser, type BrowserContext, type Page } from 'playwright';
|
||||
import { WebServer } from '../src/web/server.js';
|
||||
|
||||
const PORT = 3211;
|
||||
const BASE_URL = `http://localhost:${PORT}`;
|
||||
|
||||
let server: WebServer;
|
||||
let browser: Browser;
|
||||
|
||||
async function freshPage(): Promise<{ context: BrowserContext; page: Page }> {
|
||||
const context = await browser.newContext({
|
||||
viewport: { width: 1280, height: 800 },
|
||||
});
|
||||
const page = await context.newPage();
|
||||
return { context, page };
|
||||
}
|
||||
|
||||
async function navigateAndWait(page: Page): Promise<void> {
|
||||
await page.goto(BASE_URL, { waitUntil: 'domcontentloaded' });
|
||||
await page.waitForFunction(() => document.body.classList.contains('app-loaded'), {
|
||||
timeout: 5000,
|
||||
});
|
||||
}
|
||||
|
||||
beforeAll(async () => {
|
||||
server = new WebServer(PORT, false, true); // testMode
|
||||
await server.start();
|
||||
browser = await chromium.launch({ headless: true });
|
||||
}, 30_000);
|
||||
|
||||
afterAll(async () => {
|
||||
await browser?.close();
|
||||
await server?.stop();
|
||||
}, 30_000);
|
||||
|
||||
describe('OpenCode session initial resize', () => {
|
||||
let context: BrowserContext;
|
||||
let page: Page;
|
||||
|
||||
afterAll(async () => {
|
||||
await context?.close();
|
||||
});
|
||||
|
||||
it('selectSession is not bypassed when runOpenCode sets activeSessionId', async () => {
|
||||
// This test verifies at the code level that runOpenCode does NOT
|
||||
// pre-set activeSessionId before calling selectSession.
|
||||
// If it did, selectSession would early-return and skip sendResize.
|
||||
({ context, page } = await freshPage());
|
||||
await navigateAndWait(page);
|
||||
|
||||
// Read the runOpenCode source from the live app and verify
|
||||
// it doesn't assign activeSessionId before selectSession
|
||||
const hasPreAssignment = await page.evaluate(() => {
|
||||
const app = (window as unknown as { app: { runOpenCode: { toString: () => string } } }).app;
|
||||
const source = app.runOpenCode.toString();
|
||||
|
||||
// Check: the source should NOT have activeSessionId = ... before selectSession
|
||||
// Find positions of both patterns
|
||||
const assignIdx = source.indexOf('this.activeSessionId = data.sessionId');
|
||||
const selectIdx = source.indexOf('this.selectSession(data.sessionId)');
|
||||
|
||||
// If assign doesn't exist at all, that's the correct fix
|
||||
if (assignIdx === -1) return false;
|
||||
|
||||
// If assign comes before select, that's the bug
|
||||
return assignIdx < selectIdx;
|
||||
});
|
||||
|
||||
expect(hasPreAssignment).toBe(false);
|
||||
});
|
||||
|
||||
it('sends resize to server after creating a session via quick-start', async () => {
|
||||
({ context, page } = await freshPage());
|
||||
await navigateAndWait(page);
|
||||
|
||||
// Intercept resize API calls to track when they happen
|
||||
const resizeCalls: Array<{ url: string; cols: number; rows: number }> = [];
|
||||
await page.route('**/api/sessions/*/resize', async (route) => {
|
||||
const request = route.request();
|
||||
const body = request.postDataJSON();
|
||||
resizeCalls.push({
|
||||
url: request.url(),
|
||||
cols: body.cols,
|
||||
rows: body.rows,
|
||||
});
|
||||
// Let the request through to the server
|
||||
await route.continue();
|
||||
});
|
||||
|
||||
// Create a session via API (simulating what quick-start does)
|
||||
const sessionId = await page.evaluate(async () => {
|
||||
const res = await fetch('/api/sessions', {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ workingDir: '/tmp', name: 'oc-resize-test' }),
|
||||
});
|
||||
const data = await res.json();
|
||||
return data.id ?? data.session?.id;
|
||||
});
|
||||
|
||||
expect(sessionId).toBeTruthy();
|
||||
|
||||
// Call selectSession (which is what runOpenCode does after fix)
|
||||
await page.evaluate(async (sid: string) => {
|
||||
const app = (window as unknown as { app: { selectSession: (id: string) => Promise<void> } }).app;
|
||||
await app.selectSession(sid);
|
||||
}, sessionId);
|
||||
|
||||
// Wait for the resize to be sent (it's fire-and-forget in selectSession)
|
||||
await page.waitForTimeout(500);
|
||||
|
||||
// Verify resize was called with reasonable dimensions (not 120x40 default)
|
||||
expect(resizeCalls.length).toBeGreaterThanOrEqual(1);
|
||||
const lastResize = resizeCalls[resizeCalls.length - 1];
|
||||
expect(lastResize.url).toContain(sessionId);
|
||||
// Browser viewport is 1280x800 — terminal cols/rows should be substantially
|
||||
// different from the hardcoded 120x40 default. xterm.js calculates these
|
||||
// from container dimensions and cell size, but in headless mode with a
|
||||
// 1280x800 viewport, we should get something reasonable (>= 40 cols).
|
||||
expect(lastResize.cols).toBeGreaterThanOrEqual(40);
|
||||
expect(lastResize.rows).toBeGreaterThanOrEqual(10);
|
||||
|
||||
console.log(`[opencode-resize] resize sent: ${lastResize.cols}x${lastResize.rows}`);
|
||||
|
||||
// Cleanup
|
||||
await page.evaluate(async (sid: string) => {
|
||||
await fetch(`/api/sessions/${sid}`, { method: 'DELETE' });
|
||||
}, sessionId);
|
||||
});
|
||||
|
||||
it('selectSession does NOT early-return for a new session', async () => {
|
||||
({ context, page } = await freshPage());
|
||||
await navigateAndWait(page);
|
||||
|
||||
// Create a session
|
||||
const sessionId = await page.evaluate(async () => {
|
||||
const res = await fetch('/api/sessions', {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ workingDir: '/tmp', name: 'oc-earlyret-test' }),
|
||||
});
|
||||
const data = await res.json();
|
||||
return data.id ?? data.session?.id;
|
||||
});
|
||||
|
||||
expect(sessionId).toBeTruthy();
|
||||
|
||||
// Verify activeSessionId is NOT the new session before selectSession
|
||||
const activeBeforeSelect = await page.evaluate(() => {
|
||||
const app = (window as unknown as { app: { activeSessionId: string | null } }).app;
|
||||
return app.activeSessionId;
|
||||
});
|
||||
|
||||
// activeSessionId should be null or empty (welcome screen) — not our session
|
||||
expect(activeBeforeSelect).not.toBe(sessionId);
|
||||
|
||||
// Now call selectSession and verify it actually runs (sets activeSessionId)
|
||||
await page.evaluate(async (sid: string) => {
|
||||
const app = (window as unknown as { app: { selectSession: (id: string) => Promise<void> } }).app;
|
||||
await app.selectSession(sid);
|
||||
}, sessionId);
|
||||
|
||||
const activeAfterSelect = await page.evaluate(() => {
|
||||
const app = (window as unknown as { app: { activeSessionId: string | null } }).app;
|
||||
return app.activeSessionId;
|
||||
});
|
||||
|
||||
expect(activeAfterSelect).toBe(sessionId);
|
||||
|
||||
// Cleanup
|
||||
await page.evaluate(async (sid: string) => {
|
||||
await fetch(`/api/sessions/${sid}`, { method: 'DELETE' });
|
||||
}, sessionId);
|
||||
});
|
||||
|
||||
it('needsRefresh handler includes sendResize call', async () => {
|
||||
// The needsRefresh handler is registered inside a closure (connectSSE),
|
||||
// so we can't directly invoke it from tests. Instead, verify that the
|
||||
// handler source code dispatched to the EventSource includes sendResize.
|
||||
// This is a structural test — if the handler code changes, this test
|
||||
// ensures the resize call is preserved.
|
||||
({ context, page } = await freshPage());
|
||||
await navigateAndWait(page);
|
||||
|
||||
// Dispatch a needsRefresh event on the EventSource and intercept
|
||||
// the resulting resize API call
|
||||
const sessionId = await page.evaluate(async () => {
|
||||
const res = await fetch('/api/sessions', {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ workingDir: '/tmp', name: 'oc-refresh-test' }),
|
||||
});
|
||||
const data = await res.json();
|
||||
return data.id ?? data.session?.id;
|
||||
});
|
||||
|
||||
expect(sessionId).toBeTruthy();
|
||||
|
||||
// Select the session first so activeSessionId is set
|
||||
await page.evaluate(async (sid: string) => {
|
||||
const app = (window as unknown as { app: { selectSession: (id: string) => Promise<void> } }).app;
|
||||
await app.selectSession(sid);
|
||||
}, sessionId);
|
||||
|
||||
await page.waitForTimeout(300);
|
||||
|
||||
// Intercept resize calls
|
||||
const resizeCalls: Array<{ url: string }> = [];
|
||||
await page.route('**/api/sessions/*/resize', async (route) => {
|
||||
resizeCalls.push({ url: route.request().url() });
|
||||
await route.continue();
|
||||
});
|
||||
|
||||
// Dispatch the needsRefresh event directly on the EventSource
|
||||
// (this is how the server sends SSE events — as named events)
|
||||
await page.evaluate((sid: string) => {
|
||||
const app = (window as unknown as { app: { eventSource: EventSource } }).app;
|
||||
if (app.eventSource) {
|
||||
const event = new MessageEvent('session:needsRefresh', {
|
||||
data: JSON.stringify({ id: sid }),
|
||||
});
|
||||
app.eventSource.dispatchEvent(event);
|
||||
}
|
||||
}, sessionId);
|
||||
|
||||
// Wait for the async handler (fetches /terminal buffer + sends resize)
|
||||
await page.waitForTimeout(1500);
|
||||
|
||||
// Verify resize was called
|
||||
expect(resizeCalls.length).toBeGreaterThanOrEqual(1);
|
||||
console.log(`[opencode-resize] needsRefresh triggered ${resizeCalls.length} resize call(s)`);
|
||||
|
||||
// Cleanup
|
||||
await page.route('**/api/sessions/*/resize', (route) => route.continue());
|
||||
await page.evaluate(async (sid: string) => {
|
||||
await fetch(`/api/sessions/${sid}`, { method: 'DELETE' });
|
||||
}, sessionId);
|
||||
});
|
||||
});
|
||||
|
||||
describe('OpenCode close modal text', () => {
|
||||
let context: BrowserContext;
|
||||
let page: Page;
|
||||
|
||||
afterAll(async () => {
|
||||
await context?.close();
|
||||
});
|
||||
|
||||
it('shows "Kill Tmux & OpenCode" for opencode sessions', async () => {
|
||||
({ context, page } = await freshPage());
|
||||
await navigateAndWait(page);
|
||||
|
||||
// Create a session and mark it as opencode mode
|
||||
const sessionId = await page.evaluate(async () => {
|
||||
const res = await fetch('/api/sessions', {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ workingDir: '/tmp', name: 'oc-close-test', mode: 'opencode' }),
|
||||
});
|
||||
const data = await res.json();
|
||||
return data.id ?? data.session?.id;
|
||||
});
|
||||
|
||||
expect(sessionId).toBeTruthy();
|
||||
|
||||
// Wait for SSE to propagate the session
|
||||
await page.waitForTimeout(500);
|
||||
|
||||
// Open the close confirmation modal
|
||||
await page.evaluate((sid: string) => {
|
||||
const app = (window as unknown as { app: { requestCloseSession: (id: string) => void } }).app;
|
||||
app.requestCloseSession(sid);
|
||||
}, sessionId);
|
||||
|
||||
// Check the kill button text
|
||||
const killTitle = await page.locator('#closeConfirmKillTitle').textContent();
|
||||
expect(killTitle).toBe('Kill Tmux & OpenCode');
|
||||
|
||||
// Close the modal
|
||||
await page.evaluate(() => {
|
||||
const app = (window as unknown as { app: { cancelCloseSession: () => void } }).app;
|
||||
app.cancelCloseSession();
|
||||
});
|
||||
|
||||
// Cleanup
|
||||
await page.evaluate(async (sid: string) => {
|
||||
await fetch(`/api/sessions/${sid}`, { method: 'DELETE' });
|
||||
}, sessionId);
|
||||
});
|
||||
|
||||
it('shows "Kill Tmux & Claude Code" for claude sessions', async () => {
|
||||
({ context, page } = await freshPage());
|
||||
await navigateAndWait(page);
|
||||
|
||||
// Create a standard claude session
|
||||
const sessionId = await page.evaluate(async () => {
|
||||
const res = await fetch('/api/sessions', {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ workingDir: '/tmp', name: 'cc-close-test' }),
|
||||
});
|
||||
const data = await res.json();
|
||||
return data.id ?? data.session?.id;
|
||||
});
|
||||
|
||||
expect(sessionId).toBeTruthy();
|
||||
|
||||
await page.waitForTimeout(500);
|
||||
|
||||
// Open the close confirmation modal
|
||||
await page.evaluate((sid: string) => {
|
||||
const app = (window as unknown as { app: { requestCloseSession: (id: string) => void } }).app;
|
||||
app.requestCloseSession(sid);
|
||||
}, sessionId);
|
||||
|
||||
// Check the kill button text
|
||||
const killTitle = await page.locator('#closeConfirmKillTitle').textContent();
|
||||
expect(killTitle).toBe('Kill Tmux & Claude Code');
|
||||
|
||||
// Close the modal
|
||||
await page.evaluate(() => {
|
||||
const app = (window as unknown as { app: { cancelCloseSession: () => void } }).app;
|
||||
app.cancelCloseSession();
|
||||
});
|
||||
|
||||
// Cleanup
|
||||
await page.evaluate(async (sid: string) => {
|
||||
await fetch(`/api/sessions/${sid}`, { method: 'DELETE' });
|
||||
}, sessionId);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user