mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-09-30 12:39:42 +02:00
fix(omp): a fresh "Run OMP" click no longer silently resumes an old conversation
Found live 2026-08-27 by Tim: clicking Run OMP to start a brand-new session in a case directory with prior omp history launched --resume <old-id> instead of a clean `omp` invocation. Root cause: Session._resolvedOmpRespawnConfig() resolves-and-pins the newest on-disk omp conversation as a side effect on this._ompConfig. That is correct when reattaching to an ALREADY-TRACKED mux session (a dead-pane respawn, or a boot-recovery reattach - the constructor sets _muxSession from persisted state before startInteractive() ever runs there), but it ran unconditionally. startInteractive() computes `respawnPaneOptions: this._buildRespawnPaneOptions()` eagerly in the same object literal that builds `createSessionOptions.ompConfig: this._ompConfig`, so for a genuinely brand-new session (no muxSession in its create config, _muxSession still null) the resolve-and-pin side effect ran and poisoned this._ompConfig before that field was even read. Fix: gate the resolve-and-pin logic on `this._muxSession` already being set. A fresh session has no muxSession yet and now passes through untouched; a real reattach (muxSession present since construction) keeps resolving and pinning exactly as before. Verified live in production against the exact reported scenario (a fresh omp session in a case dir with 8+ hours of prior omp history) - confirmed both via the API (ompConfig stays empty, claudeSessionId equals the session's own id) and visually in the GUI. Regression test constructs a real Session + TmuxManager to exercise the actual private-method interaction directly, since no existing test called startInteractive() at all. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
1829fe91af
commit
ab83d8ffec
@@ -1674,6 +1674,19 @@ export class Session extends EventEmitter {
|
||||
private _resolvedOmpRespawnConfig(): OmpConfig | undefined {
|
||||
if (this.mode !== 'omp') return this._ompConfig;
|
||||
if (this._ompConfig?.resumeSessionId) return this._ompConfig;
|
||||
// Resolving-and-pinning is only correct when a mux session ALREADY exists for
|
||||
// this Session object — a dead-pane respawn, or a boot-recovery reattach (the
|
||||
// constructor sets _muxSession from persisted state before startInteractive()
|
||||
// ever runs there). A genuinely brand-new session (Run OMP -> POST
|
||||
// /api/quick-start -> a fresh Session with no muxSession in its create config)
|
||||
// has _muxSession still null at this point. Without this guard, the eager
|
||||
// `respawnPaneOptions: this._buildRespawnPaneOptions()` in startInteractive()
|
||||
// mutates this._ompConfig via the side effect below BEFORE
|
||||
// createSessionOptions.ompConfig is even read in the SAME object literal, so a
|
||||
// fresh "Run OMP" click silently inherited whatever omp conversation happened
|
||||
// to be newest on disk for this working directory instead of starting clean
|
||||
// (reported live 2026-08-27).
|
||||
if (!this._muxSession) return this._ompConfig;
|
||||
const resolvedId = findLatestOmpSessionId(this.workingDir);
|
||||
if (resolvedId) {
|
||||
this._ompConfig = { ...this._ompConfig, resumeSessionId: resolvedId };
|
||||
|
||||
@@ -0,0 +1,87 @@
|
||||
/**
|
||||
* @fileoverview Pins the "Run OMP always resumes" bug found live 2026-08-27.
|
||||
*
|
||||
* Session._resolvedOmpRespawnConfig() resolves-and-pins the newest on-disk omp
|
||||
* conversation as a side effect on `this._ompConfig`. That is correct when
|
||||
* reattaching to an ALREADY-TRACKED mux session (a dead-pane respawn, or a
|
||||
* boot-recovery reattach — the constructor sets `_muxSession` from persisted
|
||||
* state before startInteractive() ever runs there). It is wrong for a
|
||||
* genuinely brand-new session: startInteractive() computes
|
||||
* `respawnPaneOptions: this._buildRespawnPaneOptions()` EAGERLY in the same
|
||||
* object literal that builds `createSessionOptions.ompConfig: this._ompConfig`,
|
||||
* so the resolve-and-pin side effect ran and poisoned `this._ompConfig` before
|
||||
* that field was even read — a fresh "Run OMP" click in a working directory
|
||||
* with any prior omp history silently launched `--resume <old-id>` instead of
|
||||
* a clean `omp` invocation.
|
||||
*/
|
||||
import { mkdirSync, rmSync, writeFileSync } from 'node:fs';
|
||||
import { homedir } from 'node:os';
|
||||
import { join } from 'node:path';
|
||||
import { afterEach, describe, expect, it } from 'vitest';
|
||||
import { Session } from '../src/session.js';
|
||||
import { TmuxManager } from '../src/tmux-manager.js';
|
||||
import type { MuxSession } from '../src/types.js';
|
||||
|
||||
describe('OMP: fresh session vs. reattach must not share resumeSessionId resolution', () => {
|
||||
const workingDir = join(homedir(), 'codeman-cases', 'resume-test');
|
||||
const sessionDir = join(homedir(), '.omp', 'agent', 'sessions', '-codeman-cases-resume-test');
|
||||
const sessions: Session[] = [];
|
||||
|
||||
afterEach(() => {
|
||||
for (const s of sessions.splice(0)) s.stop();
|
||||
rmSync(join(homedir(), '.omp'), { recursive: true, force: true });
|
||||
});
|
||||
|
||||
function seedOmpSessionFile(id: string) {
|
||||
mkdirSync(workingDir, { recursive: true });
|
||||
mkdirSync(sessionDir, { recursive: true });
|
||||
writeFileSync(join(sessionDir, `2026-08-27T17-31-08-001Z_${id}.jsonl`), '{}');
|
||||
}
|
||||
|
||||
it('a brand-new session (no prior mux session) never inherits an on-disk conversation', async () => {
|
||||
seedOmpSessionFile('old-conversation-id');
|
||||
|
||||
const session = new Session({
|
||||
workingDir,
|
||||
mode: 'omp',
|
||||
mux: new TmuxManager(),
|
||||
useMux: true,
|
||||
});
|
||||
sessions.push(session);
|
||||
|
||||
await session.startInteractive();
|
||||
const state = session.toState();
|
||||
|
||||
expect(state.ompConfig).toBeUndefined();
|
||||
expect(session.claudeSessionId).toBe(session.id);
|
||||
});
|
||||
|
||||
it('a reattach to an existing tracked mux session still resolves and pins the real id', async () => {
|
||||
seedOmpSessionFile('real-omp-uuid');
|
||||
|
||||
const muxSession: MuxSession = {
|
||||
sessionId: 'placeholder',
|
||||
muxName: 'codeman-deadbeef',
|
||||
pid: 1,
|
||||
createdAt: Date.now(),
|
||||
workingDir,
|
||||
mode: 'omp',
|
||||
attached: false,
|
||||
};
|
||||
|
||||
const session = new Session({
|
||||
workingDir,
|
||||
mode: 'omp',
|
||||
mux: new TmuxManager(),
|
||||
useMux: true,
|
||||
muxSession,
|
||||
});
|
||||
sessions.push(session);
|
||||
|
||||
await session.startInteractive();
|
||||
const state = session.toState();
|
||||
|
||||
expect(state.ompConfig?.resumeSessionId).toBe('real-omp-uuid');
|
||||
expect(session.claudeSessionId).toBe('real-omp-uuid');
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user