fix(codex): resume stored provider sessions#2223
Conversation
Greptile SummaryThis PR fixes Codex resume by capturing the provider-native session UUID at runtime (via a new
Confidence Score: 5/5Safe to merge. The resume logic is correct for all tested provider configurations, and the session-ID capture path is isolated behind a try/catch that degrades gracefully to the existing --last fallback. The four-branch resume logic is well-exercised by the new tests, all edge cases (stored session ID, no session ID, custom flag, no flag at all) are covered, and the DB write in setProviderSessionId is idempotent. No files require special attention. set-provider-session-id.ts does an un-transacted read-modify-write, but this is consistent with the rest of the config-blob pattern in this codebase and the race window is negligibly small.
|
| Filename | Overview |
|---|---|
| src/main/core/conversations/impl/agent-command.ts | Adds providerSessionId parameter and refactors the resume branch into four ordered cases; correctly prevents the internal UUID from reaching external providers with sessionIdOnResumeOnly. |
| src/main/core/agent-hooks/agent-hook-service.ts | Adds persistCodexSessionStart to handle session-start hook events before the normal enrichment pipeline; correct early-return and try/catch pattern. |
| src/main/core/conversations/set-provider-session-id.ts | New idempotent helper that persists providerSessionId into the conversation config blob; read-then-write without a transaction means concurrent session-start events could race, though data-loss risk is very low in practice. |
| src/shared/agent-provider-registry.ts | Adds resumeWithoutSessionFlag to the type and reclassifies Codex to use positional session-ID resume; the sessionIdFlag: ' ' sentinel is unconventional but works correctly as a truthiness signal. |
| src/main/core/agent-hooks/hook-config.ts | Registers the new SessionStart hook entry via the same buildHookEntries merge pattern used for other Codex hooks. |
| src/main/core/agent-hooks/codex-session-id.ts | Small, well-tested extractor that tries session_id, resource_id, resourceId, and sessionId in priority order. |
| src/main/core/agent-hooks/agent-notify-command.ts | Adds makeCodexSessionStartHookCommand via a shared makeCodexStdinHookPostCommand helper that reads stdin or $1 on POSIX; Windows path is stdin-only, consistent with existing hook commands. |
| src/main/core/conversations/utils.ts | Extracts parseConversationConfig helper and maps providerSessionId out of the config blob in mapConversationRowToConversation. |
Sequence Diagram
sequenceDiagram
participant Codex
participant HookServer
participant AgentHookService
participant enrichEvent
participant setProviderSessionId
participant DB
participant EventBus
Codex->>HookServer: POST /hook (X-Emdash-Event-Type: session-start)
HookServer->>AgentHookService: handler(raw)
AgentHookService->>AgentHookService: "raw.type === 'session-start'?"
AgentHookService->>enrichEvent: "enrichEvent({...raw, type:'session-start'})"
enrichEvent->>DB: "SELECT taskId,projectId FROM conversations WHERE id=conversationId"
DB-->>enrichEvent: "{taskId, projectId}"
enrichEvent-->>AgentHookService: AgentEvent (providerId, conversationId, ...)
AgentHookService->>AgentHookService: "providerId === 'codex'?"
AgentHookService->>AgentHookService: extractCodexProviderSessionId(body)
AgentHookService->>setProviderSessionId: setProviderSessionId(conversationId, uuid)
setProviderSessionId->>DB: "SELECT config WHERE id=conversationId"
DB-->>setProviderSessionId: "{config}"
setProviderSessionId->>DB: "UPDATE conversations SET config={...config, providerSessionId}"
DB-->>setProviderSessionId: ok
setProviderSessionId-->>AgentHookService: true (updated)
AgentHookService->>EventBus: "emit conversationChangedChannel {providerSessionId}"
Note over Codex,EventBus: On resume
Codex->>AgentHookService: user resumes conversation
AgentHookService->>DB: load conversation (providerSessionId from config)
DB-->>AgentHookService: "Conversation{providerSessionId:'019c95f6-...'}"
AgentHookService->>AgentHookService: "buildAgentCommand({providerSessionId})"
Note right of AgentHookService: args = ['resume', '019c95f6-...']
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
src/main/core/agent-hooks/agent-hook-service.ts:82-83
`enrichEvent` already parses `raw.body` internally (in `normalizePayload`), and the result is discarded here. The body is then parsed a second time on the very next line. Consider parsing once and sharing the result — or, if the second parse exists only to keep a typed reference, a single parse suffices.
```suggestion
const body: Record<string, unknown> = raw.body
? (JSON.parse(raw.body) as Record<string, unknown>)
: {};
const providerSessionId = extractCodexProviderSessionId(body);
```
Reviews (2): Last reviewed commit: "fix(codex): respect resume fallback conf..." | Re-trigger Greptile
summary