ReviewOS

also looking at this

stacks/uplink

feat(engine): answer texts with Codex as well as Claude Code

#7
Merged glennmichael123 wants to merge feat/codex-engine into main
18 files +1080 -142
9 days ago

Closes most of #4, and carries one security fix that is unrelated to it but was found while reading the filter.

The security fix, first, because it is the part that matters today

classify() never read message.service. With Text Message Forwarding on, which is the default once an iPhone is paired and is the setup this product assumes, a forwarded SMS lands in chat.db as a direct chat whose identifier is the sender's number. That satisfied every check in the filter: direct chat, allowed handle, matching sender.

SMS sender IDs are spoofable through commodity bulk-SMS gateways. iMessage sender IDs are not. So anyone who knew the owner's phone number could send one SMS and reach an agent running with approvals disabled in the owner's home directory. It is now iMessage only, which is also the only service that reaches the Mac over satellite, so nothing real is given up.

80a7c82 is standalone and can be cherry-picked if the rest of this needs more review.

Codex support

Engine was one method, run, so everything else an agent CLI needs to say about itself was written longhand elsewhere, per CLI. It now carries id, label, authFailureHint and probe(), and Claude's stream shape, auth regex and argv stay inside ClaudeEngine.

  • CodexEngine drives codex exec.
  • UPLINK_ENGINE and a settings.json field select the engine. UPLINK_MODEL keeps meaning Claude's model alias, so an existing .env does not change meaning.
  • The menubar's step 2 follows the selection: its name, its instructions, and a picker. Codex asks for nothing on a Mac already signed in, because codex login keeps its own credential.
  • uplink:doctor emits one check per engine. Only the selected one can fail the command, so a Codex user is not shown a red cross for never having signed in to Claude. Same rule gates the popover's readiness.
  • The auth-failure text a person receives comes from the engine, instead of naming the claude CLI and a .env the downloadable app does not have.
  • The duplicate one-turn Claude probe in doctor.ts and desktop-agent.ts is gone. Both call engine.probe().
  • uplink:ask --engine <name> runs one prompt either way without editing .env.

What was verified against the binary, not the docs

Flags were read out of @openai/codex@0.158.0's clap tables and then checked live against the codex-cli 0.155 that the ChatGPT desktop app ships at /Applications/ChatGPT.app/Contents/Resources/codex. Four things would each have broken a naive port:

  1. Resume is a subcommand and rejects --cd and --sandbox, with "unexpected argument" and exit 2. The first draft of this branch pushed --sandbox after resume, which would have failed for every non-default permission mode and hidden from casual QA. A non-bypass sandbox now goes through -c sandbox_mode=, which both forms accept.
  2. There is no --append-system-prompt, so Uplink's instructions are prepended to the prompt.
  3. --skip-git-repo-check is mandatory, not optional. The default workdir is the home directory, which is not a repository, and Codex refuses to run outside one. Without it every text that names no path would fail.
  4. The JSONL flag is not stable across versions: --json on 0.155, experimental-json in 0.158's clap table. The wrong one fails silently, because the CLI then prints prose and there is nothing to parse. It is resolved once per binary from codex exec --help.

Codex reports tokens and no dollar figure, so costCents stays null rather than carrying a number invented from a price table.

findCodex() probes the ChatGPT.app path above. That is where the working binary lives on a Mac with ChatGPT installed, and it is not on PATH, so a search without it reports Codex missing on a machine that has it.

Testing

Please treat CI as the first real run of the engine tests.

17 tests pass locally: the filter (including two new cases for the SMS rejection), config, settings and engine selection. The Codex engine tests are written but could not be executed in the authoring environment, where every test that spawns a subprocess and reads its stdout times out, including three pre-existing ClaudeEngine tests on unmodified main. That is environmental rather than a repository problem, but it means these assertions are unproven:

./pantry/.bin/bun test tests/unit/uplink

./buddy lint and ./buddy typecheck could not complete there for the same reason. Every module was exercised through a live import instead, and all three generated argv forms were confirmed to parse against the real binary.

Not in this PR

From #4, still open:

  • Per-thread selection: claude and codex as control words, plus an engine column on conversations and runs.
  • That column is blocked by SqliteStore having no migration path. It creates its tables with CREATE TABLE IF NOT EXISTS (app/Uplink/sqlite-store.ts#L10), so a new column never appears in an existing uplink.sqlite and the first insert after an update fails for every existing user. That needs fixing before the column lands, not after.
  • Falling back to the other engine when one is rate limited.

🤖 Generated with Claude Code

18 changed files on the files tab, with 0 review threads.

Sign in to comment on this pull request.