Skip to content

fix(codex): resume stored provider sessions#2223

Open
janburzinski wants to merge 2 commits into
mainfrom
emdash/codex-resume-80m3p
Open

fix(codex): resume stored provider sessions#2223
janburzinski wants to merge 2 commits into
mainfrom
emdash/codex-resume-80m3p

Conversation

@janburzinski
Copy link
Copy Markdown
Collaborator

summary

  • fixes the codex resume issues

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 25, 2026

Greptile Summary

This PR fixes Codex resume by capturing the provider-native session UUID at runtime (via a new SessionStart hook) and using it to construct the codex resume <uuid> command instead of always falling back to --last. It also generalises the resume-without-session path so a resumeWithoutSessionFlag in user config is honoured rather than the registry value, and removes the old bug where the internal Emdash UUID could be passed to an external provider on resume.

  • New SessionStart hook: HookConfigWriter now registers a Codex SessionStart hook that POSTs the session JSON body to Emdash, where AgentHookService.persistCodexSessionStart extracts the session ID via extractCodexProviderSessionId and persists it to the conversation config.
  • Revised resume logic in buildAgentCommand: Four ordered branches now cover (1) resume with stored provider session ID, (2) resume with internal UUID for non-sessionIdOnResumeOnly providers, (3) resumeWithoutSessionFlag fallback, and (4) bare resumeFlag. Codex is reclassified to sessionIdFlag: ' ' / sessionIdOnResumeOnly: true / resumeWithoutSessionFlag: 'resume --last' to fit the new model.
  • setProviderSessionId: New helper that does an idempotent read-then-write to persist providerSessionId inside the conversation JSON config blob, and emits a conversationChangedChannel event so the frontend stays in sync.

Confidence Score: 5/5

Safe 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.

Important Files Changed

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-...']
Loading
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

Comment thread src/main/core/conversations/impl/agent-command.ts Outdated
Comment thread src/main/core/conversations/impl/agent-command.ts Outdated
@janburzinski
Copy link
Copy Markdown
Collaborator Author

@greptileai

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant