Skip to content

Commit a984839

Browse files
RoastFried-RFRoastFried-RF
committed
fix(plugin): close upstream OPEN backlog — openai#59 state migration, openai#113 / openai#238 / openai#75
Resolves the four genuinely-OPEN fork-affected upstream items from docs/upstream-tracking/2026-05-20-open-vs-fixed-matrix.md (handoff B2). - openai#59 FIXED — state cross-read/write. Manual port of upstream PR openai#125. `resolveStateDir` now migrates state written to the tmpdir fallback (a `/codex:*` Bash command run without CLAUDE_PLUGIN_DATA) into the persistent plugin-data dir when CLAUDE_PLUGIN_DATA is set, rewriting absolute path references in state.json + jobs/*.json. Previously the two contexts saw different state dirs and jobs appeared lost across them. Adds a migration test; also wraps the tmpdir-fallback test with a CLAUDE_PLUGIN_DATA guard (fixes the TROUBLESHOOTING #14 local flakiness). - openai#113 FIXED (docs) — commands/setup.md gains Windows install error handling: a garbled/mojibake `npm install` stderr on a non-UTF-8 console must not be reported as failure; the setup rerun is the source of truth. - openai#238 FIXED (docs) — README FAQ explains the nine `disable-model-invocation` commands (why the assistant cannot auto-invoke them) and the workaround (run them yourself, or use `/codex:rescue` for assistant-driven work). - openai#75 DOCUMENTED — a full host-permission ↔ Codex-approval bridge is a size-L design change; TROUBLESHOOTING #15 now documents the limitation (host `.claude/settings.json` deny rules are separate from the plugin's Codex approval system) so users govern Codex via `--sandbox` / approvals. Also updates the open-vs-fixed matrix (11 FIXED / 1 DOCUMENTED / 1 PARTIAL / 0 OPEN) and handoff ultraplan §5 B2. Verified: full suite 332 tests green (clean env, openai#59 state change no regression).
1 parent 0451401 commit a984839

7 files changed

Lines changed: 176 additions & 18 deletions

File tree

‎README.md‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -445,3 +445,15 @@ Yes. If you already use Codex, the plugin picks up the same [configuration](#com
445445
Yes. Because the plugin uses your local Codex CLI, your existing sign-in method and config still apply.
446446

447447
If you need to point the built-in OpenAI provider at a different endpoint, set `openai_base_url` in your [Codex config](https://developers.openai.com/codex/config-advanced/#config-and-state-locations).
448+
449+
### Why can't Claude run `/codex:status` (or `/codex:review`, `/codex:cancel`, …) on its own?
450+
451+
Nine commands — `/codex:review`, `/codex:adversarial-review`, `/codex:agent`, `/codex:continue`, `/codex:status`, `/codex:result`, `/codex:cancel`, `/codex:approve`, `/codex:deny` — are marked `disable-model-invocation: true`. Claude Code's harness will not let the assistant auto-invoke them mid-reasoning; **only you (the human) can type them**.
452+
453+
This is deliberate: these commands start or steer Codex runs (which cost tokens), mutate job state, or gate session-end review. Letting the assistant fire them autonomously could burn budget or take side-effecting actions without your explicit intent.
454+
455+
What this means in practice:
456+
457+
- To act on a Codex job, **run the command yourself** (e.g. type `/codex:status`), then ask Claude — it can read the printed output and reason about it.
458+
- For work you *do* want Claude to drive autonomously, use **`/codex:rescue`** — it is model-invocable (it delegates through the `codex:codex-rescue` subagent via the Agent tool) and is the intended entry point for assistant-driven Codex delegation.
459+
- There is no flag to flip this per-session; the policy is set in each command's frontmatter by design.

‎docs/TROUBLESHOOTING.md‎

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ For BREAKING changes from v1.x, see [MIGRATION_v2.0.md](MIGRATION_v2.0.md) first
2323
| Plugin sessions burying real chats in Codex Desktop | [#11 Codex Desktop history pollution](#11-codex-desktop-history-pollution) |
2424
| `/codex:setup` reports `loggedIn: false` even though `codex login` succeeded | [#12 Plugin loggedIn false after codex login (v2.0.0 home isolation)](#12-plugin-says-loggedin-false-after-a-successful-codex-login-v200-home-isolation) |
2525
| `node --test` fails ~5 tests locally that pass in CI | [#14 Local test failures inside a Claude Code session](#14-local-node---test-fails-5-tests-inside-a-claude-code-session) |
26+
| Host `.claude/settings.json` deny rule does not block a Codex action | [#15 Codex approvals separate from host deny rules](#15-codex-approvals-are-separate-from-host-claudesettingsjson-deny-rules-75) |
2627

2728
---
2829

@@ -491,6 +492,41 @@ failure is a real defect or this interference.
491492

492493
---
493494

495+
## 15. Codex approvals are separate from host `.claude/settings.json` deny rules (#75)
496+
497+
**Symptom**: You added a `permissions.deny` rule (or an `Edit`/`Bash` deny
498+
pattern) to your project or user `.claude/settings.json`, expecting it to
499+
also block the matching action when Codex runs it through this plugin — but
500+
Codex still performs (or still prompts for) that action.
501+
502+
**Cause**: this is a **known design limitation**, not a bug. The plugin runs
503+
Codex as a local subprocess with its **own** approval system
504+
(`plugins/codex/scripts/lib/approvals.mjs`): a hard-deny ruleset plus the
505+
interactive `/codex:approve` / `/codex:deny` flow, scoped by the Codex
506+
`--sandbox` / `--approval` settings. That system is **not bridged** to Claude
507+
Code's host `.claude/settings.json` `permissions.deny` rules. The two
508+
permission models are independent:
509+
510+
- A command/path you denied for **Claude** is *not* automatically denied for
511+
**Codex** tool calls.
512+
- The plugin's own hard-deny rules (e.g. broad recursive deletes) and
513+
workspace-scoped path grants apply to Codex regardless of host settings.
514+
515+
**What to do**: do not treat host `.claude/settings.json` deny rules as a
516+
safety boundary for Codex subprocess actions. Govern Codex instead with:
517+
518+
- the `--sandbox` setting (`read-only` / `workspace-write` /
519+
`danger-full-access`) — the primary blast-radius control;
520+
- the `--approval` mode and the `/codex:approve` / `/codex:deny` prompts —
521+
review each approval request rather than auto-approving;
522+
- the plugin's hard-deny rules for outright-dangerous operations.
523+
524+
A full host-permission ↔ Codex-approval bridge is a sizable design change
525+
tracked as upstream issue #75 and fork backlog item B2; until it lands, the
526+
two systems remain separate by design.
527+
528+
---
529+
494530
Known investigations in progress (not yet fixed in v2.0.0, no plugin-side mitigation yet):
495531

496532
- #295 `CreateProcessAsUserW failed: 1920` on Windows + sandbox=elevated
@@ -501,7 +537,7 @@ Known investigations in progress (not yet fixed in v2.0.0, no plugin-side mitiga
501537

502538
## A. Diagnostic data to gather for the spike-grade open issues
503539

504-
Sections #1-#14 ship plugin-side fixes / mitigations. The remaining items in the "Known investigations" list are **spike-grade** — root cause is in the OS layer, the codex CLI, or an upstream protocol, and the fix needs a dedicated investigation we have not yet been able to run. While that work is pending, the most useful thing a reporter can do is capture diagnostic data the next investigation can replay against. This section enumerates what to capture per issue so the eventual fix lands faster.
540+
Sections #1-#15 ship plugin-side fixes / mitigations / documented limitations. The remaining items in the "Known investigations" list are **spike-grade** — root cause is in the OS layer, the codex CLI, or an upstream protocol, and the fix needs a dedicated investigation we have not yet been able to run. While that work is pending, the most useful thing a reporter can do is capture diagnostic data the next investigation can replay against. This section enumerates what to capture per issue so the eventual fix lands faster.
505541

506542
### #295 — Windows `CreateProcessAsUserW failed: 1920`
507543

‎docs/ultraplan/2026-05-20-codex-plugin-cc-handoff-ultraplan.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ codex-plugin-cc/
134134
| ID | 항목 | 상태 | 비고 / 의도된 종료상태 |
135135
|---|---|---|---|
136136
| B1 | upstream Tier 2 (13 MEDIUM) **평가 + cherry-pick/수동포트** | deferred | scope/policy 검토 필요. 각 건을 cherry-pick vs 수동포트 vs 기각으로 판정 후 진행 |
137-
| B2 | fork-affected 실제 OPEN: **#59** state cross-rw, **#75** permission-deny bridge, **#113** install stderr decode, **#238** disable-model-invocation docs | deferred | `docs/upstream-tracking/2026-05-20-open-vs-fixed-matrix.md` 로 재검증 완료 — `#23 ANSI` 는 이미 FIXED(목록 제거), `#250` 은 PARTIAL(per-turn watchdog 가 상위 bound). 진짜 OPEN 은 이 4건만 |
137+
| B2 | fork-affected OPEN 4건 (#59·#75·#113·#238) | **거의 완료** | #59 FIXED(upstream PR #125 port), #113·#238 FIXED(docs), #75 DOCUMENTED(limitation 명시 — full bridge 는 size-L 백로그 잔존). 잔존: #75 bridge, #250 per-tool timeout(우선순위 낮음). 상세: `docs/upstream-tracking/2026-05-20-open-vs-fixed-matrix.md` |
138138
| B3 | codex-plugin-cc `/code-review` LOW 미해결 | open | (a) `invalidateTaskSession` — wire 하거나 제거 (현재 half-wired, **먼저 조사 후 결정**) (b) 신규 코드 테스트 추가 — auto capsule key·path-containment·secret-refusal 케이스 (동작은 이미 존재, 테스트만 부재) — `docs/code-review/2026-05-19-184449.md` |
139139
| B4 | **마켓플레이스 게시** | **미도달 마일스톤** | §2.1 — 실제 제출만 금지, 준비도 점검은 허용 |
140140
| B5 | 운영-모델 UltraPlan 2건 구현 | plan 존재, 미착수 | ① `2026-05-18-...token-efficiency` — Codex 호출 토큰 효율화 ② `2026-05-20-...competitive-pair` — Claude×Codex 경쟁 페어 운영 모델 (7-PR 로드맵 내장). 둘 다 L+ 규모, 사용자 우선순위 결정 필요 |

‎docs/upstream-tracking/2026-05-20-open-vs-fixed-matrix.md‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,18 +16,18 @@
1616
| (A1) | approval-loop unbounded hang | **FIXED** | 본 세션 — `waitForApprovalDecision` timeout (`CODEX_PLUGIN_APPROVAL_WAIT_MS`, 기본 30 min) |
1717
| (A3) | broker teardown zombie | **FIXED** | 본 세션 — `teardownBrokerSession` 기본 `terminateProcessTree` killer |
1818
| #250 | per-tool timeout | **PARTIAL** | 전용 per-tool timeout 없음 (`codex.mjs` grep 0). 단 #312 per-turn watchdog(A2 기본 on) + finalizing-phase 5 min bound 이 silent-tool hang 을 상위에서 bound — per-tool 세분화는 미구현 |
19-
| #59 | state cross-read/write | **OPEN** | `state.mjs` 는 `CLAUDE_PLUGIN_DATA`/temp fallback 만, 워크스페이스 간 cross-read/write 없음 |
20-
| #75 | permission-deny bridge | **OPEN** | `approvals.mjs` 의 approval 이 host `.claude/settings.json` deny rule 과 분리 — 별도 권한 시스템. bridge 또는 limitation 명시 필요 |
21-
| #113 | install stderr decode | **OPEN** | `commands/setup.md` 에 install stderr decode 처리 부재 |
22-
| #238 | disable-model-invocation 문서 | **OPEN (docs)** | feature 자체는 9개 커맨드에 적용됨. 미흡한 것은 README 의 workaround 설명 — 코드 아닌 docs 갭 |
19+
| #59 | state cross-read/write | **FIXED** | upstream PR #125 manual port — `resolveStateDir` 가 tmpdir fallback state 를 plugin-data dir 로 자동 migrate + JSON 경로 rewrite. `state.test.mjs` 에 migration 테스트 추가 |
20+
| #75 | permission-deny bridge | **DOCUMENTED** | full bridge 는 size-L design. matrix 가 제시한 "limitation 명시" 채택 — `TROUBLESHOOTING.md` #15 에 host `.claude/settings.json` deny ↔ Codex approval 분리를 명문화. bridge 구현은 백로그 잔존 |
21+
| #113 | install stderr decode | **FIXED (docs)** | `commands/setup.md` 에 Windows mojibake install stderr 처리 추가 — garbled stderr 를 실패로 오판 말고 rerun 을 SoT 로 |
22+
| #238 | disable-model-invocation 문서 | **FIXED (docs)** | `README.md` FAQ 에 9개 `disable-model-invocation` 커맨드 설명 + workaround(`/codex:rescue`) 추가 |
2323

2424
## 요약
2525

26-
- **FIXED (8)**: #312·#190·#289·#290·#314·#24/#311/#23 + A1 + A3 — handoff §5 B2 가 deferred 로 나열한 `#23 ANSI`·env sanitization·git `--end-of-options`·UTF-8 truncation 은 **이미 해결됨**. B2 목록에서 제거 대상.
27-
- **PARTIAL (1)**: #250 — 상위 bound 존재, per-tool 세분화만 미구현. 우선순위 낮음.
28-
- **OPEN (4)**: #59 state cross-rw / #75 permission-deny bridge / #113 install stderr decode / #238 docs — 진짜 미수정. 이것만 B2 후속 작업 대상.
26+
- **FIXED (11)**: #312·#190·#289·#290·#314·#24/#311/#23 + A1 + A3 + **#59 + #113 + #238**
27+
- **DOCUMENTED (1)**: #75 — limitation 명시 완료, full bridge 는 백로그 잔존
28+
- **PARTIAL (1)**: #250 — 상위 bound 존재, per-tool 세분화만 미구현. 우선순위 낮음
29+
- **OPEN (0)**: B2 의 진짜 미수정 항목 전부 처리됨
2930

30-
## handoff ultraplan §5 B2 보정 권고
31+
## handoff ultraplan §5 B2 — 처리 완료
3132

32-
B2 의 deferred 후보 목록 `(#23 ANSI, #59, #75, #113, #238, #250)` →
33-
**`#23` 제거(FIXED), `#250` PARTIAL 표기, 실제 OPEN 은 `#59·#75·#113·#238` 4건.**
33+
B2 의 OPEN 4건 (#59·#75·#113·#238) 전부 본 cycle 에서 처리. 향후 잔존 작업은 **#75 full permission-deny bridge (size L)** 와 **#250 per-tool timeout 세분화 (우선순위 낮음)** 뿐.

‎plugins/codex/commands/setup.md‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,18 @@ If the result says Codex is unavailable and npm is available:
2222
npm install -g @openai/codex
2323
```
2424

25+
Windows install error handling (#113):
26+
27+
- The `npm install` stderr on a Windows non-UTF-8 console (CP-949 / CP-1252
28+
etc.) often comes back **mojibake / garbled bytes**. Do **not** report
29+
garbled install output as a failure from the raw bytes alone.
30+
- The rerun below is the source of truth. If it reports Codex **available**,
31+
the install succeeded despite the unreadable stderr — proceed normally.
32+
- Only treat it as a real failure if the rerun still reports Codex
33+
**unavailable**. In that case surface the install command's exit code
34+
(not the garbled text) and advise the user to run
35+
`npm install -g @openai/codex` manually in a UTF-8 terminal.
36+
2537
- Then rerun:
2638

2739
```bash

‎plugins/codex/scripts/lib/state.mjs‎

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,60 @@ export function resolveStateDir(cwd) {
4747
const slugSource = path.basename(workspaceRoot) || "workspace";
4848
const slug = slugSource.replace(/[^a-zA-Z0-9._-]+/g, "-").replace(/^-+|-+$/g, "") || "workspace";
4949
const hash = createHash("sha256").update(canonicalWorkspaceRoot).digest("hex").slice(0, 16);
50+
const dirName = `${slug}-${hash}`;
5051
const pluginDataDir = process.env[PLUGIN_DATA_ENV];
51-
const stateRoot = pluginDataDir ? path.join(pluginDataDir, "state") : FALLBACK_STATE_ROOT_DIR;
52-
return path.join(stateRoot, `${slug}-${hash}`);
52+
53+
// #59 fix (manual port of upstream PR #125) — when CLAUDE_PLUGIN_DATA is
54+
// set the persistent state dir is authoritative, but a Bash command run
55+
// without that env (the common `/codex:*` invocation outside a hook) writes
56+
// to the tmpdir fallback instead. Without this migration the two contexts
57+
// see different state dirs and jobs appear lost across them. If state
58+
// exists ONLY in the tmpdir fallback, migrate it into the persistent
59+
// location so future reads/writes converge and state survives tmp cleanup.
60+
if (pluginDataDir) {
61+
const primaryDir = path.join(pluginDataDir, "state", dirName);
62+
const fallbackDir = path.join(FALLBACK_STATE_ROOT_DIR, dirName);
63+
if (
64+
!fs.existsSync(path.join(primaryDir, STATE_FILE_NAME)) &&
65+
fs.existsSync(path.join(fallbackDir, STATE_FILE_NAME))
66+
) {
67+
fs.cpSync(fallbackDir, primaryDir, { recursive: true });
68+
// Rewrite paths in all migrated JSON files (state.json + jobs/*.json)
69+
// so logFile and other absolute references point to the persistent
70+
// location. Replace both raw paths and JSON-escaped paths (Windows
71+
// backslashes are doubled inside JSON strings).
72+
const escapedFallback = fallbackDir.replaceAll("\\", "\\\\");
73+
const escapedPrimary = primaryDir.replaceAll("\\", "\\\\");
74+
const rewritePaths = (filePath) => {
75+
try {
76+
let updated = fs.readFileSync(filePath, "utf8");
77+
const original = updated;
78+
updated = updated.replaceAll(fallbackDir, primaryDir);
79+
if (escapedFallback !== fallbackDir) {
80+
updated = updated.replaceAll(escapedFallback, escapedPrimary);
81+
}
82+
if (updated !== original) {
83+
fs.writeFileSync(filePath, updated, "utf8");
84+
}
85+
} catch {
86+
/* non-fatal — a migrated file that cannot be rewritten still works
87+
for state reads; only absolute path references degrade. */
88+
}
89+
};
90+
rewritePaths(path.join(primaryDir, STATE_FILE_NAME));
91+
const jobsDir = path.join(primaryDir, JOBS_DIR_NAME);
92+
if (fs.existsSync(jobsDir)) {
93+
for (const entry of fs.readdirSync(jobsDir)) {
94+
if (entry.endsWith(".json")) {
95+
rewritePaths(path.join(jobsDir, entry));
96+
}
97+
}
98+
}
99+
}
100+
return primaryDir;
101+
}
102+
103+
return path.join(FALLBACK_STATE_ROOT_DIR, dirName);
53104
}
54105

55106
export function resolveStateFile(cwd) {

‎tests/state.test.mjs‎

Lines changed: 51 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,58 @@ import {
2020

2121
test("resolveStateDir uses a temp-backed per-workspace directory", () => {
2222
const workspace = makeTempDir();
23-
const stateDir = resolveStateDir(workspace);
23+
// #59 fix (PR #125) — this test asserts the tmpdir fallback, so it must
24+
// run with CLAUDE_PLUGIN_DATA unset. The Claude Code harness injects that
25+
// env for installed plugins, so without this guard the test fails locally
26+
// (see docs/TROUBLESHOOTING.md #14).
27+
const previousPluginDataDir = process.env.CLAUDE_PLUGIN_DATA;
28+
delete process.env.CLAUDE_PLUGIN_DATA;
29+
30+
try {
31+
const stateDir = resolveStateDir(workspace);
32+
33+
assert.equal(stateDir.startsWith(os.tmpdir()), true);
34+
assert.match(path.basename(stateDir), /.+-[a-f0-9]{16}$/);
35+
assert.match(stateDir, new RegExp(`^${os.tmpdir().replace(/[.*+?^${}()|[\]\\]/g, "\\$&")}`));
36+
} finally {
37+
if (previousPluginDataDir == null) {
38+
delete process.env.CLAUDE_PLUGIN_DATA;
39+
} else {
40+
process.env.CLAUDE_PLUGIN_DATA = previousPluginDataDir;
41+
}
42+
}
43+
});
2444

25-
assert.equal(stateDir.startsWith(os.tmpdir()), true);
26-
assert.match(path.basename(stateDir), /.+-[a-f0-9]{16}$/);
27-
assert.match(stateDir, new RegExp(`^${os.tmpdir().replace(/[.*+?^${}()|[\]\\]/g, "\\$&")}`));
45+
test("resolveStateDir migrates tmpdir state to the plugin data dir", () => {
46+
const workspace = makeTempDir();
47+
const pluginDataDir = makeTempDir();
48+
const previousPluginDataDir = process.env.CLAUDE_PLUGIN_DATA;
49+
delete process.env.CLAUDE_PLUGIN_DATA;
50+
51+
// Write state to the tmpdir fallback (simulates a /codex:* Bash command
52+
// run without CLAUDE_PLUGIN_DATA).
53+
const fallbackStateDir = resolveStateDir(workspace);
54+
const fallbackStateFile = resolveStateFile(workspace);
55+
fs.mkdirSync(fallbackStateDir, { recursive: true });
56+
const stateContent = JSON.stringify({ version: 1, config: { stopReviewGate: true }, jobs: [] });
57+
fs.writeFileSync(fallbackStateFile, `${stateContent}\n`, "utf8");
58+
59+
// Now set CLAUDE_PLUGIN_DATA (simulates a subsequent hook context).
60+
process.env.CLAUDE_PLUGIN_DATA = pluginDataDir;
61+
62+
try {
63+
const stateDir = resolveStateDir(workspace);
64+
assert.equal(stateDir.startsWith(path.join(pluginDataDir, "state")), true);
65+
assert.equal(fs.existsSync(path.join(stateDir, "state.json")), true);
66+
const migrated = JSON.parse(fs.readFileSync(path.join(stateDir, "state.json"), "utf8"));
67+
assert.equal(migrated.config.stopReviewGate, true);
68+
} finally {
69+
if (previousPluginDataDir == null) {
70+
delete process.env.CLAUDE_PLUGIN_DATA;
71+
} else {
72+
process.env.CLAUDE_PLUGIN_DATA = previousPluginDataDir;
73+
}
74+
}
2875
});
2976

3077
test("resolveStateDir uses CLAUDE_PLUGIN_DATA when it is provided", () => {

0 commit comments

Comments
 (0)