mirror of
https://github.com/stablyai/orca.git
synced 2026-10-19 08:03:13 +00:00
fix(codex): require the rescan ranking inputs and pin the tier-3 order
Review follow-up on the ranking change:
- selectedAccountCodexHome / systemCodexHomePath were optional, so a future
caller that forgot them would silently degrade to pure path order — the
accident the ranking exists to remove. Both are now required, and the
selection arrives as a thunk so the common provenance-present resume no
longer stats the account ownership marker for a ranking it never runs.
- Pin the tier-3 order in a test: with the id absent from the system home, a
per-account home beats the shared mirror ('codex-accounts' sorts before
'codex-runtime-home'). Deliberate, and now asserted rather than incidental.
- Add a drive-letter Windows fixture; join('/Users', ...) is not drive
absolute, so the existing cases took the POSIX branch even on Windows CI.
This commit is contained in:
@@ -10,6 +10,12 @@ import {
|
||||
|
||||
const tempRoots: string[] = []
|
||||
|
||||
// Why: the ranking inputs are required by design; cases below that never reach the rescan stay neutral.
|
||||
const withoutHomeRanking = {
|
||||
getSelectedAccountCodexHome: (): string | null => null,
|
||||
systemCodexHomePath: null
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
for (const root of tempRoots.splice(0)) {
|
||||
rmSync(root, { recursive: true, force: true })
|
||||
@@ -116,7 +122,8 @@ describe('resolveTrustedCodexSessionResumeHome', () => {
|
||||
findTrustedCodexSessionResume({
|
||||
sessionId: 'session-a',
|
||||
transcriptPath: plainPath,
|
||||
trustedCodexHomes: [homePath]
|
||||
trustedCodexHomes: [homePath],
|
||||
...withoutHomeRanking
|
||||
})
|
||||
).resolves.toEqual({ homePath, transcriptPath: compressedPath })
|
||||
|
||||
@@ -125,7 +132,8 @@ describe('resolveTrustedCodexSessionResumeHome', () => {
|
||||
findTrustedCodexSessionResume({
|
||||
sessionId: 'session-a',
|
||||
transcriptPath: compressedPath,
|
||||
trustedCodexHomes: [homePath]
|
||||
trustedCodexHomes: [homePath],
|
||||
...withoutHomeRanking
|
||||
})
|
||||
).resolves.toEqual({ homePath, transcriptPath: plainPath })
|
||||
})
|
||||
@@ -149,7 +157,8 @@ describe('resolveTrustedCodexSessionResumeHome', () => {
|
||||
findTrustedCodexSessionResume({
|
||||
sessionId,
|
||||
transcriptPath: undefined,
|
||||
trustedCodexHomes: [homePath]
|
||||
trustedCodexHomes: [homePath],
|
||||
...withoutHomeRanking
|
||||
})
|
||||
).resolves.toEqual({ homePath, transcriptPath: compressedPath })
|
||||
})
|
||||
@@ -169,6 +178,7 @@ describe('resolveTrustedCodexSessionResumeHome', () => {
|
||||
sessionId,
|
||||
transcriptPath: undefined,
|
||||
trustedCodexHomes: ['/Users/example/.codex', '/managed/account/home'],
|
||||
...withoutHomeRanking,
|
||||
listSessionFiles
|
||||
})
|
||||
).resolves.toEqual({ homePath: '/managed/account/home', transcriptPath: rolloutPath })
|
||||
@@ -186,6 +196,7 @@ describe('resolveTrustedCodexSessionResumeHome', () => {
|
||||
sessionId: 'session-a',
|
||||
transcriptPath,
|
||||
trustedCodexHomes: ['/managed/account/home'],
|
||||
...withoutHomeRanking,
|
||||
fileIsRegular: () => true,
|
||||
listSessionFiles
|
||||
})
|
||||
@@ -204,6 +215,7 @@ describe('resolveTrustedCodexSessionResumeHome', () => {
|
||||
sessionId,
|
||||
transcriptPath: `/managed/origin/home/sessions/2026/07/20/rollout-${sessionId}.jsonl`,
|
||||
trustedCodexHomes: ['/managed/origin/home', '/managed/other/home'],
|
||||
...withoutHomeRanking,
|
||||
fileIsRegular: () => false,
|
||||
listSessionFiles
|
||||
})
|
||||
@@ -220,6 +232,7 @@ describe('resolveTrustedCodexSessionResumeHome', () => {
|
||||
sessionId: '../session',
|
||||
transcriptPath: undefined,
|
||||
trustedCodexHomes: ['/Users/example/.codex'],
|
||||
...withoutHomeRanking,
|
||||
listSessionFiles
|
||||
})
|
||||
).resolves.toBeNull()
|
||||
@@ -241,9 +254,9 @@ describe('findTrustedCodexSessionResume legacy-rescan home ranking', () => {
|
||||
yield join(sessionsRoot, '2026', '07', '20', `rollout-2026-07-20T15-50-19-${sessionId}.jsonl`)
|
||||
}
|
||||
|
||||
const listRolloutOnlyIn = (homePath: string) =>
|
||||
const listRolloutIn = (...homePaths: string[]) =>
|
||||
async function* (sessionsRoot: string): AsyncIterable<string> {
|
||||
if (sessionsRoot === join(homePath, 'sessions')) {
|
||||
if (homePaths.some((homePath) => sessionsRoot === join(homePath, 'sessions'))) {
|
||||
yield* listRolloutInEveryHome(sessionsRoot)
|
||||
}
|
||||
}
|
||||
@@ -259,7 +272,7 @@ describe('findTrustedCodexSessionResume legacy-rescan home ranking', () => {
|
||||
sessionId,
|
||||
transcriptPath: undefined,
|
||||
trustedCodexHomes,
|
||||
selectedAccountCodexHome: accountBHome,
|
||||
getSelectedAccountCodexHome: () => accountBHome,
|
||||
systemCodexHomePath: systemHome,
|
||||
listSessionFiles: listRolloutInEveryHome
|
||||
})
|
||||
@@ -273,7 +286,7 @@ describe('findTrustedCodexSessionResume legacy-rescan home ranking', () => {
|
||||
sessionId,
|
||||
transcriptPath: undefined,
|
||||
trustedCodexHomes: [sharedMirror, accountAHome, systemHome],
|
||||
selectedAccountCodexHome: null,
|
||||
getSelectedAccountCodexHome: () => null,
|
||||
systemCodexHomePath: systemHome,
|
||||
listSessionFiles: listRolloutInEveryHome
|
||||
})
|
||||
@@ -290,7 +303,7 @@ describe('findTrustedCodexSessionResume legacy-rescan home ranking', () => {
|
||||
sessionId,
|
||||
transcriptPath: undefined,
|
||||
trustedCodexHomes,
|
||||
selectedAccountCodexHome: null,
|
||||
getSelectedAccountCodexHome: () => null,
|
||||
systemCodexHomePath: systemHome,
|
||||
listSessionFiles: listRolloutInEveryHome
|
||||
})
|
||||
@@ -298,15 +311,57 @@ describe('findTrustedCodexSessionResume legacy-rescan home ranking', () => {
|
||||
}
|
||||
})
|
||||
|
||||
it('prefers a per-account home over the shared mirror when the system home lacks the id', async () => {
|
||||
// Why: 'codex-accounts' sorts before 'codex-runtime-home', so tier 3 flips the pre-ranking
|
||||
// winner here. Deliberate: the mirror is hot-swapped and names no account, the account home does.
|
||||
await expect(
|
||||
findTrustedCodexSessionResume({
|
||||
sessionId,
|
||||
transcriptPath: undefined,
|
||||
trustedCodexHomes: [systemHome, sharedMirror, accountAHome],
|
||||
getSelectedAccountCodexHome: () => null,
|
||||
systemCodexHomePath: systemHome,
|
||||
listSessionFiles: listRolloutIn(sharedMirror, accountAHome)
|
||||
})
|
||||
).resolves.toEqual({ homePath: accountAHome, transcriptPath: rolloutIn(accountAHome) })
|
||||
})
|
||||
|
||||
it('ranks Windows homes case-insensitively and keeps the caller path spelling', async () => {
|
||||
const windowsRoot = 'C:\\Users\\Example'
|
||||
const windowsSystemHome = `${windowsRoot}\\.codex`
|
||||
const windowsAccountAHome = `${windowsRoot}\\AppData\\Roaming\\Orca\\codex-accounts\\a\\home`
|
||||
const windowsAccountBHome = `${windowsRoot}\\AppData\\Roaming\\Orca\\codex-accounts\\b\\home`
|
||||
const windowsRolloutIn = (homePath: string): string =>
|
||||
`${join(homePath, 'sessions')}\\2026\\07\\20\\rollout-2026-07-20T15-50-19-${sessionId}.jsonl`
|
||||
const listSessionFiles = async function* (sessionsRoot: string): AsyncIterable<string> {
|
||||
yield `${sessionsRoot}\\2026\\07\\20\\rollout-2026-07-20T15-50-19-${sessionId}.jsonl`
|
||||
}
|
||||
|
||||
await expect(
|
||||
findTrustedCodexSessionResume({
|
||||
sessionId,
|
||||
transcriptPath: undefined,
|
||||
trustedCodexHomes: [windowsSystemHome, windowsAccountAHome, windowsAccountBHome],
|
||||
// Why: settings and discovery can disagree on drive/segment case; the selection must still match.
|
||||
getSelectedAccountCodexHome: () => windowsAccountBHome.toLowerCase(),
|
||||
systemCodexHomePath: windowsSystemHome,
|
||||
listSessionFiles
|
||||
})
|
||||
).resolves.toEqual({
|
||||
homePath: windowsAccountBHome,
|
||||
transcriptPath: windowsRolloutIn(windowsAccountBHome)
|
||||
})
|
||||
})
|
||||
|
||||
it('still resumes the only home holding the id, selected or not', async () => {
|
||||
await expect(
|
||||
findTrustedCodexSessionResume({
|
||||
sessionId,
|
||||
transcriptPath: undefined,
|
||||
trustedCodexHomes: [systemHome, sharedMirror, accountAHome, accountBHome],
|
||||
selectedAccountCodexHome: accountAHome,
|
||||
getSelectedAccountCodexHome: () => accountAHome,
|
||||
systemCodexHomePath: systemHome,
|
||||
listSessionFiles: listRolloutOnlyIn(accountBHome)
|
||||
listSessionFiles: listRolloutIn(accountBHome)
|
||||
})
|
||||
).resolves.toEqual({ homePath: accountBHome, transcriptPath: rolloutIn(accountBHome) })
|
||||
})
|
||||
@@ -321,7 +376,7 @@ describe('findTrustedCodexSessionResume legacy-rescan home ranking', () => {
|
||||
sessionId,
|
||||
transcriptPath: rolloutIn(accountBHome),
|
||||
trustedCodexHomes: [systemHome, accountAHome, accountBHome],
|
||||
selectedAccountCodexHome: accountAHome,
|
||||
getSelectedAccountCodexHome: () => accountAHome,
|
||||
systemCodexHomePath: systemHome,
|
||||
fileIsRegular: () => false,
|
||||
listSessionFiles
|
||||
|
||||
@@ -103,13 +103,19 @@ export function claimsCodexRolloutLayout(transcriptPath: string | undefined): bo
|
||||
* they asked for. The real system home ranks next because codex refreshes it
|
||||
* directly. Everything else is ordered by normalized path so no winner ever
|
||||
* depends on the order accounts happen to sit in settings.
|
||||
*
|
||||
* Both inputs are required, not optional: a caller that forgot one would
|
||||
* silently degrade to pure path order, which is the accident this exists to
|
||||
* remove. The selection arrives as a thunk because resolving it stats the
|
||||
* account's ownership marker, and the far more common provenance-present
|
||||
* resume never reaches the ranking at all.
|
||||
*/
|
||||
function rankTrustedCodexHomesForRescan(args: {
|
||||
trustedCodexHomes: readonly string[]
|
||||
selectedAccountCodexHome?: string | null
|
||||
systemCodexHomePath?: string | null
|
||||
getSelectedAccountCodexHome: () => string | null
|
||||
systemCodexHomePath: string | null
|
||||
}): string[] {
|
||||
const selected = args.selectedAccountCodexHome?.trim()
|
||||
const selected = args.getSelectedAccountCodexHome()?.trim()
|
||||
const system = args.systemCodexHomePath?.trim()
|
||||
const selectedComparison = selected ? normalizeRuntimePathForComparison(selected) : null
|
||||
const systemComparison = system ? normalizeRuntimePathForComparison(system) : null
|
||||
@@ -139,8 +145,8 @@ export async function findTrustedCodexSessionResume(args: {
|
||||
sessionId: string
|
||||
transcriptPath: string | undefined
|
||||
trustedCodexHomes: readonly string[]
|
||||
selectedAccountCodexHome?: string | null
|
||||
systemCodexHomePath?: string | null
|
||||
getSelectedAccountCodexHome: () => string | null
|
||||
systemCodexHomePath: string | null
|
||||
fileIsRegular?: (filePath: string) => boolean
|
||||
listSessionFiles?: (sessionsRoot: string) => AsyncIterable<string>
|
||||
}): Promise<{ homePath: string; transcriptPath: string } | null> {
|
||||
|
||||
+2
-1
@@ -855,7 +855,8 @@ async function prepareCodexSessionResumeForLaunch(args: {
|
||||
trustedCodexHomes: trustedHomes,
|
||||
// Why: the legacy id rescan's winning home becomes this pane's CODEX_HOME, i.e. its account;
|
||||
// rank it by the current selection so settings insertion order can never decide the account.
|
||||
selectedAccountCodexHome: codexRuntimeHome.getSelectedHostAccountCodexHomePath(),
|
||||
// Lazy: only the legacy branch ranks, so a provenance-present resume never stats the marker.
|
||||
getSelectedAccountCodexHome: () => codexRuntimeHome!.getSelectedHostAccountCodexHomePath(),
|
||||
systemCodexHomePath: systemHomePath
|
||||
})
|
||||
if (!sessionSource) {
|
||||
|
||||
Reference in New Issue
Block a user