cursor: Fix PR #506 (Codex CLI adapter / runtime-adapter de-drift) — - #522
cursor: Fix PR #506 (Codex CLI adapter / runtime-adapter de-drift) —#522servathadi wants to merge 3 commits into
Conversation
Make the topology-A workers the reference adapter: shared client with server-derived identity, declared runtime types, fleet-attach:v1, and land-at-review rails — so later BYOA harnesses copy one contract, not bespoke glue. Co-authored-by: Cursor <cursoragent@cursor.com>
Add topology-A codex-worker that dispatches via `codex exec --sandbox --json`, reads mupot MCP over streamable-HTTP (url + bearer_token_env_var, no SSE), filters own-assignee open tasks, isolates a worktree, verifies, and lands at review — proving the contract generalizes past cursor for cursor→codex failover. Co-authored-by: Cursor <cursoragent@cursor.com>
…bed codex env Close the three P0s that blocked PR #506: attach/signature failure is now terminal before any poll/claim/dispatch; presence capabilities are derived from fleet agent_type server-side (client asserts dropped); codex exec gets a minimal allowlisted child env and danger-full-access is refused for topology-A. Conformance now runs behavioral proofs (fail-closed attach, env scrub, detach/inbox impl) instead of source-string greps alone. Co-authored-by: Cursor <cursoragent@cursor.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 01dcfd8eba
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| */ | ||
| async function derivePresenceCapabilities(env: Env, identity: string): Promise<string[]> { | ||
| const row = await env.DB.prepare( | ||
| `SELECT agent_type FROM fleet_agents WHERE tenant = ?1 AND agent_id = ?2 LIMIT 1`, |
There was a problem hiding this comment.
fleet_agents.agent_type is not server-authoritative: both attach routes accept type from the agent itself (for example, src/fleet/attach-routes.ts:181-184) and overwrite this column on every attach. An observer-capable welded agent can therefore attach with type: "builder", register in any project it can read, and receive build; pickOnlineBuilder in src/concierge/service.ts then treats it as eligible for task dispatch. Derive this tag from the canonical agent role/configuration instead of the runtime-reported fleet row.
Useful? React with 👍 / 👎.
| git("worktree", "add", "-b", branch, str(worktree), "main") | ||
| try: | ||
| ensure_codex_mcp_config(cfg.mcp_url, CODEX_MCP_ENV_VAR, write=True) | ||
| proc = codex_run(worktree, build_brief(task, worktree, branch), cfg.token) |
There was a problem hiding this comment.
When codex exec exceeds the documented TIMEOUT, subprocess.run raises TimeoutExpired here after the task has already been changed to in_progress. The outer loop only logs the exception, while future cycles poll only status=open, so that task is permanently stranded and its worktree is discarded. Catch the timeout, as mumcp-worker.py does, and report the task blocked or otherwise return it to a pollable state.
Useful? React with 👍 / 👎.
| log(f"codex exit {proc.returncode}; output tail:\n{(proc.stdout or '')[-800:]}") | ||
| ok, note = verify(worktree, branch) |
There was a problem hiding this comment.
If Codex creates a commit and then exits nonzero because of a tool/runtime failure, execution continues into verify, which checks only for a commit and a successful TypeScript compile. A partial commit can therefore be pushed, opened as a PR, and reported as verified review work despite the Codex command failing. Check proc.returncode before verification and report the task blocked on a nonzero exit.
Useful? React with 👍 / 👎.
| env = build_codex_child_env( | ||
| dict(os.environ), | ||
| mcp_env_var=CODEX_MCP_ENV_VAR, | ||
| token=token, | ||
| ) |
There was a problem hiding this comment.
The child receives the same member-capable token the trusted driver uses for task_update, even though its brief describes the MCP surface as read-only. Consequently Codex—and any prompt injection it follows—can invoke the mutation tools exposed to that token, including changing task state/body or creating work in its squad, bypassing the intended split where only the driver claims and lands tasks. Configure the child with a distinct observer/read-only credential or otherwise restrict its MCP tool surface.
Useful? React with 👍 / 👎.
| identity = boot_session( | ||
| cfg, | ||
| presence_adapter="codex", | ||
| log=log, | ||
| ) |
There was a problem hiding this comment.
This one-shot worker attaches and marks the runtime live, but neither this exit path nor the equivalent cursor/mumcp paths ever calls the newly implemented detach. Even a cycle with no tasks therefore exits while fleet_agents remains running until its liveness TTL expires; during that window src/bus/consumer.ts can select the external-inbox route and deliver dispatches to a process that no longer exists. Wrap the post-attach session in cleanup that detaches on every normal or exceptional exit.
Useful? React with 👍 / 👎.
| from cryptography.hazmat.primitives.asymmetric.ed25519 import Ed25519PrivateKey | ||
| from cryptography.hazmat.primitives.serialization import load_pem_private_key |
There was a problem hiding this comment.
Whenever CURSOR_KEY, MUMCP_KEY, or CODEX_KEY selects the documented signed path, this import requires the third-party cryptography package, but the repository has no Python dependency manifest or installation instruction for it. A normal checkout without that undeclared package raises ModuleNotFoundError during the fail-closed boot and cannot run any keyed adapter; the offline conformance currently misses this because it mocks _load_ed25519_private_key. Declare and install the dependency or implement signing with an already provisioned runtime.
Useful? React with 👍 / 👎.
| if prior and f'bearer_token_env_var = "{env_var}"' in prior.group(0) and f'url =' in prior.group(0): | ||
| log(f"mcp config already present in {CODEX_CONFIG}") | ||
| return stanza |
There was a problem hiding this comment.
When an existing mupot stanza has the requested environment-variable name and any url field, this branch accepts it without comparing that URL to mcp_url. Reusing a Codex home after changing MUPOT_MCP therefore boots and claims work against the new pot while the Codex child connects to the old endpoint, potentially sending the new pot's bearer token to the previous host and giving the model context from the wrong tenant. Require the configured URL to match exactly or rewrite the stanza.
Useful? React with 👍 / 👎.
| agent = created.get("agent") if isinstance(created.get("agent"), dict) else {} | ||
| agent_id = agent.get("id") | ||
| log(f"create_agent ok id={agent_id!r} slug={agent.get('slug')!r}") | ||
| minted = mcp_call(op_cfg, "mint_agent_token", {"agent": slug, "label": f"{slug}-member"}) |
There was a problem hiding this comment.
Agent slugs are only unique within a squad, while mint_agent_token deliberately refuses an ambiguous bare slug. If another squad already contains an agent named codex, this live flow successfully creates the requested agent and then fails at token minting, leaving an orphan agent behind. Pass the agent_id returned by create_agent instead of resolving the user-selected slug globally.
Useful? React with 👍 / 👎.
servathadi
left a comment
servathadi
left a comment
There was a problem hiding this comment.
BLOCK — exact-head content gate at 01dcfd8e.
The Codex child receives the same member-capable token as the worker despite the proposed read-only boundary, and an existing MCP stanza is accepted without verifying that its URL matches the configured endpoint. A subprocess timeout is only logged by the outer loop, leaving the claimed task state stranded in progress, and one-shot workers exit without detaching presence.
These are P1 authority/lifecycle blockers. Existing exact-head threads remain unresolved; the branch is merge-conflicted and has CodeQL-only evidence rather than the required full gate suite.
|
Tracked as #551 for incremental fix (2026-07-25 backlog triage — codex diverse-gate BLOCK, real finding, not merged/live). Leaving this PR open as the fix branch; will re-request gate and merge on GREEN when picked up. |
|
Closing as dead weight: target #506 closed unmerged (never landed), so this fix-PR has nothing left to fix. Verified via |
1 participant
Dispatched to the
cursoragent (Grok) headless via the mupot loop for taskaf0369ba-96ee-4908-87ad-f5851c2de320.Task done-when: All 3 P0 fixed: (1) attach/signature failure is TERMINAL/fail-closed before any dispatch; (2) runtime capabilities server-derived not driver-asserted; (3) codex exec gets a minimal allowlisted env (no GITHUB/cloud/deploy creds), danger-full-access disallowed for topology-A. Conformance made real (exercises attach fail-closed + child-env scrub + detach/inbox impl). tsc+tests green. Re-gated Kasra-core + codex dyad.
Driver verified: cursor committed real work +
tsc --noEmitclean. Kasra-core gates this PR before merge (the task is inreview; cursor cannot self-close it).