mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 00:02:31 +00:00
fix(omp): retain recorded transcript paths when resuming (#20634)
Based on the resume-locator proposal in stablyai/orca#16276 by @CodeHourra. Retains UUID-based ownership and existing reattach behavior.
This commit is contained in:
@@ -0,0 +1,28 @@
|
||||
# OMP recorded transcript resume
|
||||
|
||||
A sleeping OMP session can retain its transcript path from a hook without an
|
||||
explicit `launchConfig.ompResumeFilePath`. Both cold-restore startup and generic
|
||||
sleeping-session launch already forward the provider metadata to
|
||||
`getAgentResumeArgv`; that builder must keep the recorded path.
|
||||
|
||||
Resolution order is explicit launch path, recorded transcript path, then UUID.
|
||||
The existing shell-aware builder quotes the selected argument for the execution
|
||||
host. An older metadata record without a path retains UUID fallback. OMP provider
|
||||
claim keys and equality remain UUID-based, so a later hook adding the path does
|
||||
not create a second automatic-resume identity.
|
||||
|
||||
`tests/tools/omp-resume-transcript-locator-smoke.mjs` creates an actual OMP session
|
||||
outside its default session store. UUID lookup fails there; the absolute path and
|
||||
Orca's generated argv resume the original session. Run it with Bun and a read-only
|
||||
OMP checkout as argv[2], under `ORCA_BACKGROUND_LAUNCH=1`. It uses a disposable home
|
||||
and makes no model requests.
|
||||
|
||||
This bounded correction follows the resume-locator portion of
|
||||
[PR #16276](https://github.com/stablyai/orca/pull/16276) by @CodeHourra. It does not
|
||||
adopt that PR's reattach injection or title changes. The reattach proposal treats
|
||||
missing snapshot/replay as permission to type a resume command, but
|
||||
`daemon-pty-spawn-result.ts` explicitly permits `isReattach: true` without a
|
||||
snapshot. That payload absence is not positive evidence of a newly created shell.
|
||||
The proposal also adds the path to OMP claim identity, which separates UUID-only
|
||||
metadata from a later path-enriched record for the same provider session. Those
|
||||
changes require separate evidence and are outside this patch's review scope.
|
||||
@@ -1,6 +1,7 @@
|
||||
import { afterEach, describe, expect, it } from 'vitest'
|
||||
import type { SleepingAgentSessionRecord } from '../../../shared/agent-session-resume'
|
||||
import { makePaneKey } from '../../../shared/stable-pane-id'
|
||||
import { getProviderSessionClaimKey } from './sleeping-agent-pane-ownership'
|
||||
import { useAppStore } from '@/store'
|
||||
import { resumeSleepingAgentSessionsForWorktree } from './resume-sleeping-agent-session'
|
||||
|
||||
@@ -248,3 +249,12 @@ describe('resume sleeping agent provider claims', () => {
|
||||
expect(resumeSleepingAgentSessionsForWorktree('wt-1')).toBe(1)
|
||||
})
|
||||
})
|
||||
|
||||
it('keeps the same OMP provider claim when a later hook supplies its transcript path', () => {
|
||||
const record = makeRecord(makePaneKey('tab-1', LEAF_ID))
|
||||
const enriched = {
|
||||
...record,
|
||||
providerSession: { ...record.providerSession, transcriptPath: '/custom/session.jsonl' }
|
||||
}
|
||||
expect(getProviderSessionClaimKey(enriched)).toBe(getProviderSessionClaimKey(record))
|
||||
})
|
||||
|
||||
@@ -78,9 +78,9 @@ const record: SleepingAgentSessionRecord = {
|
||||
updatedAt: 1
|
||||
}
|
||||
|
||||
async function launch(): Promise<string | undefined> {
|
||||
async function launch(sessionRecord = record): Promise<string | undefined> {
|
||||
const { launchSleepingAgentSession } = await import('./sleeping-agent-session-launch')
|
||||
launchSleepingAgentSession(record)
|
||||
launchSleepingAgentSession(sessionRecord)
|
||||
const options = mockCreateTab.mock.calls.at(-1)?.[3] as
|
||||
| { pendingStartup?: { command: string } }
|
||||
| undefined
|
||||
@@ -152,4 +152,36 @@ describe('launchSleepingAgentSession Windows shell quoting', () => {
|
||||
`codex '--dangerously-bypass-approvals-and-sandbox' 'resume' '${SESSION_ID}'`
|
||||
)
|
||||
})
|
||||
it.each([
|
||||
['cmd.exe', 'omp "--resume" "C:\\custom sessions\\session.jsonl"'],
|
||||
['powershell.exe', "omp '--resume' 'C:\\custom sessions\\session.jsonl'"]
|
||||
])('keeps a hook-only OMP locator when waking into %s', async (shell, expected) => {
|
||||
store.settings.terminalWindowsShell = shell
|
||||
const omp: SleepingAgentSessionRecord = {
|
||||
...record,
|
||||
agent: 'omp',
|
||||
providerSession: {
|
||||
key: 'session_id',
|
||||
id: SESSION_ID,
|
||||
transcriptPath: 'C:\\custom sessions\\session.jsonl'
|
||||
}
|
||||
}
|
||||
await expect(launch(omp)).resolves.toBe(expected)
|
||||
})
|
||||
it('keeps the remote OMP path instead of using the local Windows shell or UUID', async () => {
|
||||
store.settings.terminalWindowsShell = 'cmd.exe'
|
||||
store.repos = [{ id: 'repo-1', connectionId: 'ssh-1', path: '/repo' }]
|
||||
const omp: SleepingAgentSessionRecord = {
|
||||
...record,
|
||||
agent: 'omp',
|
||||
providerSession: {
|
||||
key: 'session_id',
|
||||
id: SESSION_ID,
|
||||
transcriptPath: '/remote/custom sessions/session.jsonl'
|
||||
}
|
||||
}
|
||||
await expect(launch(omp)).resolves.toBe(
|
||||
"omp '--resume' '/remote/custom sessions/session.jsonl'"
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -167,3 +167,34 @@ describe('agent session resume metadata', () => {
|
||||
).toEqual({ key: 'session_id', id: 'ok' })
|
||||
})
|
||||
})
|
||||
|
||||
describe('OMP recorded resume locators', () => {
|
||||
it.each([
|
||||
{
|
||||
explicit: '/explicit/session.jsonl',
|
||||
recorded: '/hook/session.jsonl',
|
||||
target: '/explicit/session.jsonl'
|
||||
},
|
||||
{ explicit: undefined, recorded: ' /hook/session.jsonl ', target: '/hook/session.jsonl' },
|
||||
{ explicit: ' ', recorded: '/hook/session.jsonl', target: '/hook/session.jsonl' },
|
||||
{ explicit: undefined, recorded: ' ', target: 'session-id' },
|
||||
{ explicit: undefined, recorded: undefined, target: 'session-id' }
|
||||
])('selects explicit then recorded path then UUID %j', ({ explicit, recorded, target }) => {
|
||||
expect(
|
||||
getAgentResumeArgv(
|
||||
'omp',
|
||||
{ key: 'session_id', id: 'session-id', transcriptPath: recorded },
|
||||
explicit
|
||||
)
|
||||
).toEqual(['omp', '--resume', target])
|
||||
})
|
||||
it('retains UUID equality when hook path metadata arrives later', () => {
|
||||
expect(
|
||||
agentProviderSessionsEqual(
|
||||
'omp',
|
||||
{ key: 'session_id', id: 'session-id' },
|
||||
{ key: 'session_id', id: 'session-id', transcriptPath: '/hook/session.jsonl' }
|
||||
)
|
||||
).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -278,7 +278,11 @@ export function getAgentResumeArgv(
|
||||
return providerSession.key === 'session_id' ? ['devin', '--resume', id] : null
|
||||
case 'omp':
|
||||
return providerSession.key === 'session_id'
|
||||
? ['omp', '--resume', ompResumeFilePath?.trim() || id]
|
||||
? [
|
||||
'omp',
|
||||
'--resume',
|
||||
ompResumeFilePath?.trim() || providerSession.transcriptPath?.trim() || id
|
||||
]
|
||||
: null
|
||||
// Why: the joined form is the only one Copilot documents, and it matches the
|
||||
// flag spelling buildAgentResumeInvocation bakes into persisted AI Vault
|
||||
|
||||
@@ -0,0 +1,81 @@
|
||||
// Bun; argv[2] is a read-only OMP checkout. No model requests.
|
||||
import assert from 'node:assert/strict'
|
||||
import { mkdtemp, mkdir, rm } from 'node:fs/promises'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join, resolve } from 'node:path'
|
||||
import { pathToFileURL } from 'node:url'
|
||||
import { getAgentResumeArgv } from '../../src/shared/agent-session-resume.ts'
|
||||
|
||||
assert.ok(process.argv[2], 'Pass a read-only OMP checkout path')
|
||||
const scratch = await mkdtemp(join(tmpdir(), 'orca-omp-resume-locator-'))
|
||||
process.env.HOME = join(scratch, 'home')
|
||||
process.env.USERPROFILE = process.env.HOME
|
||||
for (const key of [
|
||||
'OMP_CODING_AGENT_DIR',
|
||||
'PI_CODING_AGENT_DIR',
|
||||
'PI_CONFIG_DIR',
|
||||
'OMP_PROFILE',
|
||||
'PI_PROFILE',
|
||||
'PI_CONFIG_FILES'
|
||||
]) {
|
||||
delete process.env[key]
|
||||
}
|
||||
process.env.XDG_CONFIG_HOME = join(scratch, 'config')
|
||||
process.env.XDG_DATA_HOME = join(scratch, 'data')
|
||||
process.env.XDG_STATE_HOME = join(scratch, 'state')
|
||||
const source = (name) =>
|
||||
pathToFileURL(join(resolve(process.argv[2]), 'packages/coding-agent/src', name)).href
|
||||
const managers = []
|
||||
try {
|
||||
await mkdir(process.env.HOME, { recursive: true })
|
||||
const { SessionManager } = await import(source('session/session-manager.ts'))
|
||||
const { Settings } = await import(source('config/settings.ts'))
|
||||
const { createSessionManager } = await import(source('main.ts'))
|
||||
const { parseArgs } = await import(source('cli/args.ts'))
|
||||
const cwd = join(scratch, 'folder workspace')
|
||||
await mkdir(cwd)
|
||||
const original = SessionManager.create(cwd, join(scratch, 'custom sessions'))
|
||||
managers.push(original)
|
||||
original.appendMessage({
|
||||
role: 'user',
|
||||
content: 'task in a custom session root',
|
||||
timestamp: Date.now()
|
||||
})
|
||||
await original.ensureOnDisk()
|
||||
await original.flush()
|
||||
const settings = await Settings.init({ cwd })
|
||||
await assert.rejects(
|
||||
createSessionManager({ resume: original.getSessionId() }, cwd, settings),
|
||||
/not found/
|
||||
)
|
||||
const direct = await createSessionManager({ resume: original.getSessionFile() }, cwd, settings)
|
||||
managers.push(direct)
|
||||
assert.equal(direct.getSessionId(), original.getSessionId())
|
||||
const argv = getAgentResumeArgv('omp', {
|
||||
key: 'session_id',
|
||||
id: original.getSessionId(),
|
||||
transcriptPath: original.getSessionFile()
|
||||
})
|
||||
assert.ok(argv)
|
||||
assert.equal(
|
||||
argv[2],
|
||||
original.getSessionFile(),
|
||||
'Orca must retain the recorded transcript locator'
|
||||
)
|
||||
const resumed = await createSessionManager(parseArgs(argv.slice(1)), cwd, settings)
|
||||
managers.push(resumed)
|
||||
assert.equal(resumed.getSessionId(), original.getSessionId())
|
||||
console.log(
|
||||
JSON.stringify({
|
||||
customRootUuidMisses: true,
|
||||
absolutePathResumes: true,
|
||||
generatedArgvResumes: true,
|
||||
modelCalls: 0
|
||||
})
|
||||
)
|
||||
} finally {
|
||||
for (const manager of managers) {
|
||||
await manager?.close()
|
||||
}
|
||||
await rm(scratch, { recursive: true, force: true })
|
||||
}
|
||||
Reference in New Issue
Block a user