Skip to content

fix(setup): add a command to refresh the Codex ACP bridge - #259

Merged
david-hummingbot merged 5 commits into
hummingbot:mainfrom
mlguys:fix/codex-acp-bridge
Sep 30, 2026
Merged

david-hummingbot merged 5 commits into
hummingbot:mainfrom
mlguys:fix/codex-acp-bridge

Conversation

@mlguys

@mlguys mlguys commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

An outdated Codex ACP bridge can reject newer models even after the standalone Codex CLI has been updated, because the bridge bundles its own CLI. Add make refresh-codex to explicitly update the global bridge and its bundled Codex dependency when this happens.

The command installs the current published bridge, prints the installed bridge and Codex versions, and instructs the operator to start a new Codex session. It preserves npm already on PATH, otherwise loads nvm, and runs both steps in the same shell. It is listed in make help and the README. Session launching stays unchanged: no per-session update checks, version pins, or changes to the child environment are introduced.

Validation:

  • make help, make -n refresh-codex, and git diff --check passed.
  • Isolated shell checks passed for npm already on PATH, the nvm fallback, a clear error when npm is unavailable, and stopping the remaining steps after an install failure.
  • The refresh recipe ran successfully and reported bridge 2.0.1 with bundled Codex 0.159.2.
  • A real Condor ACPClient smoke test selected gpt-6.1-sol, received one pwd tool call and its completion event, and received the final READY response with end_turn.

@greptile-apps

greptile-apps Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[Low risk] Adds a new make command for updating a development tool.

The PR appears safe to merge; the remaining launch-command test gap is non-blocking.

Findings

  1. P2 Launch command lacks coverage ▶

Summary

The PR adds an operator-run command to refresh the Codex ACP bridge and documents when to use it.

  • The latest revision makes the command load nvm when npm is absent, keeps installation and version reporting in one shell, and stops after an install failure.

Reviews (4) · Last reviewed commit: "fix(make): load nvm when the Codex refre..."

Comment thread condor/acp/client.py Outdated
Comment thread condor/acp/client.py Outdated
Comment thread tests/test_model_readiness.py Outdated
Comment on lines +157 to +167
def test_codex_latest_bridge_readiness_parses_flags_and_tag(monkeypatch):
package = "@agentclientprotocol/codex-acp"
monkeypatch.setattr(readiness, "acp_login_state", lambda base: True)
monkeypatch.setattr(readiness, "npx_packages_installed", lambda: {package})
bridge = next(row for row in readiness.acp_bridges() if row["agent_key"] == "codex")
assert bridge["available"] is True
assert bridge["logged_in"] is True
assert (
readiness.install_command(bridge["command"])
== f"npm install -g {package}@latest"
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Launch command lacks coverage

This test checks mocked readiness and install-command text, but not the Codex command returned by resolve_acp or its launch path. A later change to the launch flags could leave the test green while undoing the bridge update. Add a focused launch-command contract test.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@mlguys mlguys changed the title fix(acp): launch the latest Codex bridge fix(setup): add a command to refresh the Codex ACP bridge Sep 30, 2026
Comment thread Makefile Outdated
@greptile-apps

greptile-apps Bot commented Sep 30, 2026

Copy link
Copy Markdown

Want your agent to iterate on Greptile's feedback? Try greploops.

@david-hummingbot
david-hummingbot merged commit 07b4601 into hummingbot:main Sep 30, 2026
5 checks passed
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.

2 participants