S0-01: Prove signed-in Codex ACP and bounded cleanup - #50
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #25
User outcome
Vellum can open a real
codex-acpsession through Kritarth's existing ChatGPT sign-in, stream a useful response, terminate withdone, and clean the adapter process tree without leaving an orphan.Scope
VELLUM_CODEX_MODELcompatibility override passed to the first-party adapter without a shell.Non-goals: no paper UI, custom authentication, raw API keys, RAG, alternate harness, renderer, or Electron UI change.
Acceptance criteria
StdioAcpClient.newSession("codex")succeeds through first-party adapter.done.Automated verification
dispose()resolved while adapter child remained alive (expected null not to be null).npm test -- core/acp/stdio-client.test.ts— 18/18 passed.npm test— 24 files, 147 tests passed. Existing jsdom canvas warnings remain in Reader tests; all Reader tests passed.npm run typechecknpm run buildgit diff --checkLive Electron verification
Not applicable: no
src/,electron/, preload, IPC, or visible UI files changed. Issue #25 requires the real headless signed-in ACP path, exercised below. No screenshots required.Visual and console evidence
No visible UI change. Legacy adapter stderr logs nonfatal model-metadata decode errors for newer
maxreasoning metadata. These are disclosed indocs/verification/codex-acp-2026-08-12.md; successful turns still end indone/end_turn.Real-paper / ACP evidence
Environment:
codex-cli 0.147.0;codex login statusreportedLogged in using ChatGPT; PATH adapter is deprecated@zed-industries/codex-acp 0.16.0with embedded Codex0.137.0; ACP SDK1.2.1. No credential files or tokens were read.Baseline
npm run smoke:acp -- codexopened a session but failed because service selectedgpt-5.6-sol, newer than embedded runtime, and left an orphan native adapter.Final independent verification on commit
71ab478ranVELLUM_CODEX_MODEL=gpt-5.5 npm run smoke:acp -- codextwice:2026-08-12T01:40:16.747Zto01:40:30.602Z; textpong; kindstool_result,text,tool_result,done;stopReason: end_turn; exit 0; immediate orphan query empty.2026-08-12T01:40:39.384Zto01:40:46.766Z; same text/kinds/stop reason; exit 0; immediate orphan query empty.Exact cleanup query:
ps -axo pid,ppid,state,lstart,command | rg '[c]odex-acp|[c]odex app-server'.Limitations and follow-ups
@zed-industries/codex-acp 0.16.0is deprecated and noisy against current model metadata. Maintained prerequisite is@agentclientprotocol/codex-acp 1.1.14; local migration remains recommended.VELLUM_CODEX_MODEL=gpt-5.5until maintained adapter/current runtime replaces the legacy package.Independent review
status:in-progress, neverstatus:blocked.