Skip to content

Commit aa37075

Browse files
konsta95codex
konsta95
and
codex
committed
fix(diff): refresh retained panes after a denied resume close
Report denied resume closes and refresh panes that remain open using the new session's timing. Ignore an old resume completion after a newer session transition so it cannot discard that session's pending refresh. Co-Authored-By: Codex <[email protected]> Codex-Session: 01a0e7e9-70a7-7722-a93f-ebb95f90c163
1 parent 7fb9aef commit aa37075

2 files changed

Lines changed: 132 additions & 2 deletions

File tree

‎mods/diff/hooks/register.ts‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1067,12 +1067,24 @@ export function register(on: On) {
10671067
}
10681068

10691069
const isResume = e.command === 'resume'
1070-
const isKeptOpen = isPaneOpen && !isResume
1070+
const epochBeforeClose = pin.epoch
10711071

10721072
if (isPaneOpen && isResume) {
1073-
await closePane(host).catch(() => undefined)
1073+
try {
1074+
await closePane(host)
1075+
} catch (error) {
1076+
host.uiLog(
1077+
`Could not close the diff panel after the session changed: ${Views.sanitizeName(messageOf(error))}`,
1078+
)
1079+
}
10741080
}
10751081

1082+
if (epochBeforeClose !== pin.epoch) {
1083+
return result
1084+
}
1085+
1086+
const isKeptOpen = isPaneOpen
1087+
10761088
unpin()
10771089
timers.get('refresh')?.cancel()
10781090
timers.delete('refresh')

‎mods/diff/tests/owner-lifecycle.test.ts‎

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -653,6 +653,124 @@ describe('owner-lifecycle', () => {
653653
expect(world.opened.map(pane => pane.id)).toEqual(['diff'])
654654
})
655655

656+
test('/resume refreshes a retained pane when its close is denied and restored history has no edits', async ($, on) => {
657+
const world = lifecycleWorld(on)
658+
await $.session.start(Fixtures.SESSION)
659+
await $.command.run(Fixtures.DIFF)
660+
await world.clock.advance(Fixtures.SETTLE_MS)
661+
expect(Fixtures.textOf(await $.ui.render(Fixtures.PANE))).toContain('app.ts')
662+
663+
Object.assign(world.script, MOVED)
664+
world.state.denyClose = true
665+
await $.command.run(Fixtures.RESUME)
666+
await world.clock.advance(Fixtures.SETTLE_MS)
667+
668+
expect(world.pane.visible).toBe(true)
669+
const text = Fixtures.textOf(await $.ui.render(Fixtures.PANE))
670+
expect(text).toContain('other.ts')
671+
expect(text).not.toContain('app.ts')
672+
expect(world.opened).toHaveLength(1)
673+
expect(await $.command.run(Fixtures.DIFF)).toEqual({ text: Names.PANEL_HIDDEN_TEXT })
674+
expect(world.pane.visible).toBe(false)
675+
})
676+
677+
test('/resume reports a denied close and reuses the retained pane for restored edits', async ($, on) => {
678+
const world = lifecycleWorld(on)
679+
await $.session.start(Fixtures.SESSION)
680+
await $.command.run(Fixtures.DIFF)
681+
await world.clock.advance(Fixtures.SETTLE_MS)
682+
683+
Object.assign(world.script, MOVED)
684+
world.state.transcript = Fixtures.EDITED_TRANSCRIPT
685+
world.state.denyClose = true
686+
await $.command.run(Fixtures.RESUME)
687+
await world.clock.advance(Fixtures.SETTLE_MS)
688+
689+
expect(world.logs.join('\n')).toContain('test close denied')
690+
expect(world.pane.visible).toBe(true)
691+
expect(world.opened).toHaveLength(1)
692+
expect(world.closed).toHaveLength(1)
693+
const text = Fixtures.textOf(await $.ui.render(Fixtures.PANE))
694+
expect(text).toContain('other.ts')
695+
expect(text).not.toContain('app.ts')
696+
})
697+
698+
test('/resume waits for its session timing before refreshing a pane whose close was denied', async ($, on) => {
699+
const world = lifecycleWorld(on)
700+
filesWrittenAt(on, 2000)
701+
world.state.startedAt = 100
702+
await $.session.start(Fixtures.SESSION)
703+
await $.command.run(Fixtures.DIFF)
704+
await world.clock.advance(5000)
705+
const before = world.reads.length
706+
707+
Object.assign(world.script, MOVED)
708+
world.state.startedAt = 5000
709+
world.state.delayUsage = 2000
710+
world.state.denyClose = true
711+
const resuming = $.command.run(Fixtures.RESUME)
712+
await world.clock.settle()
713+
expect(world.reads).toHaveLength(before)
714+
await world.clock.advance(3000)
715+
await resuming
716+
await world.clock.advance(Fixtures.SETTLE_MS)
717+
718+
const text = Fixtures.textOf(await $.ui.render(Fixtures.PANE))
719+
expect(text).toContain('No changes this session')
720+
expect(text).toContain('1 file edited before this session')
721+
})
722+
723+
test('a delayed denied resume close does not revive a pane closed by a later request', async ($, on) => {
724+
const world = lifecycleWorld(on)
725+
await $.session.start(Fixtures.SESSION)
726+
await $.command.run(Fixtures.DIFF)
727+
await world.clock.advance(Fixtures.SETTLE_MS)
728+
const before = world.reads.length
729+
world.state.delayClose = 1000
730+
world.state.denyClose = true
731+
const resuming = $.command.run(Fixtures.RESUME)
732+
await world.clock.settle()
733+
expect(world.closed).toHaveLength(1)
734+
735+
world.state.delayClose = 0
736+
expect(await $.command.run(Fixtures.DIFF)).toEqual({ text: Names.PANEL_HIDDEN_TEXT })
737+
await world.clock.advance(2000)
738+
await resuming
739+
await world.clock.advance(Fixtures.SETTLE_MS)
740+
741+
expect(world.pane.visible).toBe(false)
742+
expect(world.opened).toHaveLength(1)
743+
expect(world.reads.slice(before)).toEqual([])
744+
})
745+
746+
test('an older denied resume close cannot reset a newer clear or discard its refresh', async ($, on) => {
747+
const world = lifecycleWorld(on)
748+
filesWrittenAt(on, 2000)
749+
world.state.startedAt = 100
750+
await $.session.start(Fixtures.SESSION)
751+
await $.command.run(Fixtures.DIFF)
752+
await world.clock.advance(5000)
753+
world.state.delayClose = 1000
754+
world.state.denyClose = true
755+
const resuming = $.command.run(Fixtures.RESUME)
756+
await world.clock.settle()
757+
expect(world.closed).toHaveLength(1)
758+
759+
world.state.delayClose = 0
760+
world.state.delayRead = 2000
761+
world.state.startedAt = 5000
762+
Object.assign(world.script, MOVED)
763+
await $.command.run(Fixtures.CLEAR)
764+
await world.clock.advance(4000)
765+
await resuming
766+
await world.clock.advance(Fixtures.SETTLE_MS)
767+
768+
expect(world.pane.visible).toBe(true)
769+
const text = Fixtures.textOf(await $.ui.render(Fixtures.PANE))
770+
expect(text).toContain('No changes this session')
771+
expect(text).toContain('1 file edited before this session')
772+
})
773+
656774
test('/resume consumes a pending refresh when restored history has no edits', async ($, on) => {
657775
const world = lifecycleWorld(on)
658776
await $.session.start(Fixtures.SESSION)

0 commit comments

Comments
 (0)