Skip to content

feat: cube-codex-track-file-change#220

Merged
jacsamell merged 2 commits into
mainfrom
writer-opus/cube-codex-track-file-change
May 24, 2026
Merged

feat: cube-codex-track-file-change#220
jacsamell merged 2 commits into
mainfrom
writer-opus/cube-codex-track-file-change

Conversation

@jacsamell
Copy link
Copy Markdown
Contributor

@jacsamell jacsamell commented May 24, 2026

Autonomous implementation via Agent Cube

Writer: Writer Opus (writer_b)
Branch: writer-opus/cube-codex-track-file-change

Review decisions in .prompts/decisions/cube-codex-track-file-change-*.json

Summary

This PR introduces per-invocation tracking of file_change paths emitted by the Codex SDK to replace unreliable on-disk existence checks. Previously, the provider would check if decision files existed after streamed runs, which could fail due to stale state. Now it tracks paths emitted during each invocation and only triggers a forcing follow-up if the expected decision file wasn't written.

Changes

codex.ts (the main implementation):

  • Replaces existsSync checks with an invocation-local tracking mechanism
  • Records all file_change paths emitted during the main Codex run and any follow-up runs
  • Triggers a single forcing follow-up only when the expected decision file path wasn't observed in the recorded write set
  • Removes post-run disk checks, delegating final validation to the downstream Python adapter

codex.test.ts (new test suite):

  • Adds 4 test scenarios covering the decision-file enforcement logic
  • Tests edge cases: missing file_change events, absolute paths, relative paths, and unrelated file changes
  • Validates that forcing follow-ups are correctly triggered or suppressed based on observed writes

Review Change Stack

jacsamell and others added 2 commits May 24, 2026 10:46
…ge tracking

The previous decision-file enforcement used `existsSync` after the main
streamed run. A stale decision file from an earlier panel round would
satisfy the check, so the bridge would skip the forcing follow-up and
Cube would aggregate the stale content (observed on aetheron-connect-v2
PR #1388: Codex emitted zero file_change events but a stale decision
file existed, leading to a misleading APPROVED review).

Track `file_change` paths emitted on item.started/item.completed during
the current bridge invocation and trigger the follow-up when the
expected path is absent from the write set. Relative emitted paths are
resolved against opts.worktree; the expected path is resolved with
path.resolve so comparison is exact post-resolution. The follow-up still
runs on the same Codex thread and its events are tracked too.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The runtime adapter launches dist/cli.js directly, so the previous
commit's source change to providers/codex.ts only took effect once the
bundled artifact was regenerated.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 24, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d1c635ff-3fe3-4033-bba8-6f2430823a0b

📥 Commits

Reviewing files that changed from the base of the PR and between 89ecd6a and f63b137.

⛔ Files ignored due to path filters (1)
  • sdk-bridge/dist/cli.js is excluded by !**/dist/**
📒 Files selected for processing (2)
  • sdk-bridge/src/providers/codex.test.ts
  • sdk-bridge/src/providers/codex.ts

Walkthrough

The provider's decision-file enforcement now relies on observing file_change events emitted by Codex during execution, rather than checking on-disk existence. The expected decision-file path is resolved to an absolute path and matched against recorded event paths. A forcing follow-up runs only if the path was not observed during the main run.

Changes

Decision-file enforcement via file_change events

Layer / File(s) Summary
Decision-file enforcement refactor
sdk-bridge/src/providers/codex.ts
Import node:path instead of existsSync; add per-invocation Set to track file_change paths; resolve expectedDecisionFile to absolute path; record file_change paths while iterating main run event stream; trigger forcing follow-up only when expected path not observed in set; run one follow-up on same thread and record its file_change events without re-checking disk.
Test suite for decision-file enforcement
sdk-bridge/src/providers/codex.test.ts
Mock @openai/codex-sdk with FakeThread that captures runStreamed calls and yields scripted events; set up test scaffolding with temporary filesystem and stdout silencing; validate four scenarios: (1) no file_change events trigger forcing follow-up; (2) absolute path match suppresses follow-up; (3) relative path match suppresses follow-up; (4) mismatched path triggers follow-up with decision file reference.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • aetheronhq/agent-cube#203: Both PRs modify sdk-bridge/src/providers/codex.ts's Codex streaming/run logic—retrieved PR implements the Codex SDK provider and event handling, whilst the main PR further changes the file_change-driven "forcing follow-up" decision-file enforcement behaviour within that same run/event flow.
  • aetheronhq/agent-cube#188: Main PR's codex provider now decides whether to trigger a "forcing follow-up" by matching recorded ThreadEvent.file_change paths against the expected decision-file path, and PR #188 changes the Codex adapter/prompt to write decision files to a canonical absolute path or a worktree-relative fallback—directly impacting the file_change paths the provider observes.

Poem

🐰 A file was born from streaming light,
No disk checks needed—events set it right!
File_change whispers guide the way,
Or follow-up prompts save the day!


Comment @coderabbitai help to get the list of available commands and usage tips.

@jacsamell jacsamell merged commit c2398eb into main May 24, 2026
0 of 2 checks passed
@jacsamell jacsamell deleted the writer-opus/cube-codex-track-file-change branch May 24, 2026 01:25
jacsamell added a commit that referenced this pull request May 24, 2026
THE root cause of judge_4 (codex-sdk) failing on every cube prv 1388
across multiple days of Phase 3 dogfood. Codex's sandbox was telling
the truth: `additionalDirectories` maps to `--add-dir <path>` which
grants READ access only. WRITES to those paths get "Operation not
permitted" — codex literally cannot write to the main repo from a
worktree CWD.

PR #188 fixed this for the legacy codex.py CLI adapter by adding
`-c sandbox_workspace_write.writable_roots=[<project_root>]` via
`--config` overrides. The SDK provider never got the port.

The @openai/codex-sdk's Codex({...}) constructor accepts a `config`
field documented as: "Additional `--config key=value` overrides ...
Provide a JSON object and the SDK will flatten it into dotted paths
and serialize values as TOML literals."

Passing { sandbox_workspace_write: { writable_roots: [...] } } in
the config object reaches the codex CLI as
`-c sandbox_workspace_write.writable_roots=[...]` — same effective
shape as the legacy adapter.

Keeping `additionalDirectories` on startThread too — read access
doesn't hurt and may be the correct SDK mechanism in future versions.

Evidence: ~/.cube/logs/judge-judge_4-pr-1388-peer-review-1779582290.json
Codex's exact words: "I cannot write the exact canonical path from this
session. The sandbox blocks it with `Operation not permitted` because
`/Users/jacob/dev/aetheron-connect-v2` is outside the writable roots."

Loop disclosure: panel review was blocked by Claude weekly quota
(cube identity hit limit; resets May 25 23:30 ACST). Spec was
written, cube auto fired, all 5 judges failed with auth errors before
even reaching the writer. Shipping the 3-line fix directly because
the writer phase couldn't progress and this is a P0 dogfood blocker.
PR #220's file_change tracking + PR #218's auto-approve gate guard
provide safety net coverage in lieu of synchronous panel review;
re-review via cube auto on resume once quota resets.

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
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.

1 participant