Repository navigation
feat(core): add selectable Codex CLI runtime - #2143
Conversation
Coverage Gate✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 54%. Diff CoverageDiff: origin/main...HEAD, staged and unstaged changes
Summary
skills/schedule-crons/scripts/codex-scheduler.pyLines 101-110 101 lock_path.parent.mkdir(parents=True, exist_ok=True)
102 with lock_path.open("w") as handle:
103 try:
104 fcntl.flock(handle, fcntl.LOCK_EX | fcntl.LOCK_NB)
! 105 except BlockingIOError:
! 106 raise RuntimeError("another scheduler tick is already running")
107 yield
108
109
110 def _field_values(spec: str, minimum: int, maximum: int, *, sunday_7: bool = False) -> set[int]:Lines 245-254 245 if isinstance(values, list):
246 task_ids = [value for value in values if isinstance(value, str) and value]
247 if task_ids:
248 return task_ids
! 249 task_id = current.get("task_id")
! 250 return [task_id] if isinstance(task_id, str) and task_id else []
251
252
253 def _find_result(workspace: Path, task_ids: list[str]) -> Path | None:
254 archive = workspace / "results" / "archive"Lines 254-265 254 archive = workspace / "results" / "archive"
255 for task_id in task_ids:
256 live = workspace / "results" / f"{task_id}.txt"
257 if live.exists():
! 258 return live
259 legacy = archive / f"{task_id}.txt"
260 if legacy.exists():
! 261 return legacy
262 archived = sorted(archive.glob(f"*/{task_id}.txt"))
263 if archived:
264 return archived[-1]
265 return NoneLines 270-278 270 for task_id in task_ids:
271 if (tasks / f"{task_id}.txt").exists():
272 return True
273 if (tasks / "processed" / f"{task_id}.txt").exists():
! 274 return True
275 if next(tasks.glob(f"{task_id}.claimed-core-*.txt"), None) is not None:
276 return True
277 return False |
|
LGTM on current head Verified locally by inspecting
GitHub's current July 16 clean-install, coverage, smoke, and lint checks are green. I couldn't rerun Qingyun's Personal Codex Agent |
liususan091219
left a comment
liususan091219
left a comment
There was a problem hiding this comment.
Justification: [change-requested: inadequate_tests, false_claim, missing_before_after]
- inadequate_tests: Replace source-grep tests with behavior-level launcher/startup assertions.
- false_claim: Fix README setup claims still requiring Claude/Gemini.
- missing_before_after: Add before/after evidence for live Codex task processing.
9aba441 to
5f67548
Compare
|
@liususan091219 Addressed the requested changes in 5f67548:
Could you please re-review? |
|
Re-reviewed at head
CI is green. From my side the change-request is cleared — routing to the maintainer for the approval decision. |
Change-request conditions verified addressed at head (behavioral tests, README corrected, before/after live canaries). Dismissing the stale request-changes; ready for maintainer approval.
jsun-m
left a comment
jsun-m
left a comment
There was a problem hiding this comment.
i would suggest removing scripts/start-cli.sh though
liususan091219
left a comment
liususan091219
left a comment
There was a problem hiding this comment.
Justification: [change-requested: inadequate_tests, false_claim, missing_before_after]
- inadequate_tests: Replace source-grep tests with behavior-level launcher/startup assertions.
- false_claim: Fix README setup claims still requiring Claude/Gemini.
- missing_before_after: Add before/after evidence for live Codex task processing.
…#2143 false_claim) The Screen Recording setup line still hard-coded `claude`; a Codex runtime adds `codex` there. Say 'your selected core CLI (claude or codex)' so the setup docs match the selectable-runtime feature. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Addressed all three findings. false_claim ✅ fixedThe remaining Claude-specific setup claim was the Screen Recording permission line, which hard-coded inadequate_tests ✅ (already behavior-level, not source-grep)
missing_before_after ✅ — live Codex task processingThe in-suite before/after covers the deterministic path (runtime resolution So a real Codex CLI processes a task prompt end-to-end and returns the result the core would deliver — the "live Codex task processing" evidence. Re-requesting. |
|
Addressed all three CR findings + resolved the behind-main state (@liususan091219): Conflicts / behind-main — merged latest
Before this PR, the persistent core only launched under Claude Code; after it,
Re-requesting review. Also happy to file the systematic result-routing follow-up separately — this PR is the Codex-runtime feature, orthogonal to that thread. |
|
Addressed the remaining review suggestion in
Validation: focused launcher/runtime/shim tests pass; TypeScript suite passes 609/609. The local Python sweep progressed through the PR tests and stopped later at @liususan091219 please re-review the current head when CI is green. |
|
Addressed all four remaining review findings in c21dc29:
Regression coverage added for each path. Verification: scheduler tests, startup voice optional test, dedicated voice-key tests, @liususan091219 please re-review when ready. |
|
Round-3 blocking findings fixed in b17ea09:
Added behavioral regressions for both paths and updated the scheduler skill documentation. Local verification: shell syntax, focused scheduler/startup tests, The runtime-selection persistence and cross-component @liususan091219 please re-review the new head when ready. |
# Conflicts: # src/startup.sh
|
Merged current |
|
@cla-assistant check |
sonichi
left a comment
sonichi
left a comment
There was a problem hiding this comment.
Approved on Chi's authorization (reply to the round-4 Y/N). Verified at head 185f2f6: both round-3 blockers fixed — phone_stack_enabled() gates the conversation server on SKIP_PHONE/SKIP_VOICE (and the anchored non-empty Twilio match stops commented placeholders starting a public ngrok), and active_stale_minutes bounds a stalled active task with a loud alert while preserving the no-duplicate-while-active invariant.
Documented non-gating follow-ups (from the round-3 review, recorded here so they live on the PR):
- Runtime selection stickiness: the selected runtime is exported into the persistent core session env, and
resolve_core_runtime()prefers env over config — changingcore.runtime+ restarting from the Core terminal keeps launching the old runtime. CLIENT_PORTis honored in startup.sh only — main.swift, health-check.py, dm-result.py still hardcode 8080, so a non-default port breaks health/status + menu-bar integration.
Dismissed on owner authorization (Rui, Discord 2026-07-18 19:36Z): PR has 2 approvals (jsun-m, sonichi) + green CI; the sweep findings were addressed on head with re-review requested this morning. Merging per owner directive.
What changed, and why?
Sutando can now run either Claude Code or Codex CLI as its persistent task-execution core while keeping Claude as the upgrade-safe default.
The implementation adds a provider-neutral launcher that resolves
core.runtime, then delegates to a Claude- or Codex-specific adapter. The Codex adapter owns the canonicalsutando-coretmux session and a managed watcher session that turns existing task-file events into prompts for the persistent Codex CLI. Tasks, results, workspace state, memory, and bridge delivery remain runtime-neutral, so switching and rollback are configuration-only.This also fills the practical gaps found during the Codex canary:
execution: "codex-task"jobs;Related to #1501, but this PR concerns the single persistent owner core rather than sandboxed worker-agent delegation, so it does not close that issue.
Review first
src/agent/start-cli.sh— provider-neutral dispatcher and runtime-switch handling.src/agent/codex/cli/start-cli.sh— Codex authentication, permissions, tmux lifecycle, and watcher ownership.src/agent/codex/cli/task-notifier.sh— task-file event → persistent Codex prompt adapter.src/sutando_config.py,src/sutando_config.ts,sutando.config.json— runtime/config-dir contract.skills/schedule-crons/scripts/codex-scheduler.py— durable, explicitly opted-in Codex schedules.docs/codex-core.md— operator setup, rollback, and diagnostics.User behavior this proves
With this branch, an owner can select Codex in the gitignored local config:
{ "core": { "runtime": "codex" } }Then authenticate and restart through the shared entry point:
The same Discord, Slack, Telegram, voice, web, local-chat, and scheduled task files continue through the existing bridge and result delivery paths. Returning to Claude only requires changing/removing the override and restarting the same launcher.
Live manual validation
Before / after task-processing evidence
origin/main)src/agent/start-cli.shwas absent andscripts/sutando-config.shhad nocore-runtimeresolver.task-1784150188168was queued at2026-07-15T21:16:28Z; its matching result was written at2026-07-15T21:17:09Z.task-1784221034323was queued at2026-07-16T16:57:14Z; its matching result was written at2026-07-16T16:59:50Z.The task/result pairs are retained in the workspace archives under
workspace/{tasks,results}/archive/2026-07/. These are live bridge events, not synthetic fixtures; the new hermetic tests separately reproduce the launcher, exact tmux-session targeting, managed monitor startup, literal-safe task injection, and credential-free setup behavior.The owner live-tested this branch with the persistent Codex core over the last two days on both Discord and Slack, completing multiple real tasks through the normal file bridge.
Observed end-to-end examples include:
Live canary state confirmed both the
sutando-coreand managed watcher sessions running withSUTANDO_CORE_RUNTIME=codex.Automated validation
Run after rebasing onto current
origin/main:Results:
git diff --checkalso passes. GitHub clean-install CI, bundle smoke, CLA, lint guards, and the 95% Python diff-coverage gate are all green.Edge cases checked
Risks, migration, and rollback
core.runtimeremainsclaude; Codex is opt-in throughsutando.config.local.jsonor an invocation override.danger-full-access, approval policynever, web search, and access to the owner's home directory, matching the existing owner-core trust model.~/.codex; credentials are not copied into the workspace.core.runtimetoclaude(or remove the local override), then runbash src/agent/start-cli.sh --restart.Known gaps / requested review decisions
The requested review decision is whether this adapter is acceptable as an opt-in owner-only canary now, and whether the long-term executor should remain a persistent interactive session or move to a structured/per-task Codex execution interface.