You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
recognize Codex CLI 0.143's Use /skills to list available skills empty-composer frame
tolerate future placeholder copy changes using the input marker plus model/cwd footer
reopen the readiness safety latch after a user accepts a new directory's trust prompt
keep MCP startup authentication warnings non-blocking and preserve Claude Code behavior
Root cause
The v0.3.3 readiness fallback correctly saw and retained Codex's trust screen, but it only accepted the newer Ask Codex to do anything composer copy as proof that onboarding had cleared. Codex 0.143 renders a different empty-composer placeholder, so the session remained ready: false after trust was accepted and Agent Studio kept holding the initial prompt.
The failing Mac's live PTY replay confirmed the trust screen was in history, the current frame contained the 0.143 composer copy, and the MCP warnings did not match a blocking-screen signature.
harness unit suite excluding the sandbox-only unreadable-directory test: 2,105 passed
harness performance suite: 4 passed
harness typecheck: passed
harness lint: no errors (one pre-existing warning)
desktop tests: 152 passed
desktop typecheck and full dependency build: passed
The excluded workspace-watcher test was run separately and reproduces unchanged because this sandbox cannot make the fixture directory unreadable with chmod; it is unrelated to this patch.
No confidentiality issues: the changeset, comments and fixtures name only Sapiom, Codex/Agent Studio and public MCP server names. Tests are excluded from tsconfig.build.json, so nothing new reaches the tarball. patch is the right level.
Findings
1. Readiness is latched shut by any composer copy not in the literal list, with no escape hatch (packages/harness/src/core/adapters/codex.ts:119)
Once blockingPromptSeen is set, refreshImmediatePromptState clears it only on a positive detectReadyPrompt match, and it nulls readinessCandidateAt on every frame while latched (session-manager.ts:1176), so there is no liveness ceiling to fall back on. If a Codex build's empty composer renders neither exact phrase, the session stays ready: false forever, waitUntilReady times out and submitInput throws SessionNotReadyError — i.e. exactly the v0.3.3 bug this PR fixes, re-armed for the next copy change.
This is the second patch to the same single point of failure, and the evidence behind it is one machine's PTY replay, which cannot show whether Use /skills to list available skills is the only 0.143 empty-composer placeholder. Two concrete ways it still hangs:
0.143 rotates or conditionalises the hint (e.g. the /skills line is suppressed when no skills are installed) — then this fix works intermittently or not at all for some users on the very version it targets.
Codex 0.148+ ships new copy — same silent hang, another patch release.
Either cite the 0.143 source line that pins the placeholder set (the way BLOCKING_PROMPT_SIGNATURES cites 0.147.0 sources) and enumerate all variants, or add a version-independent proof of the composer — the › glyph plus the model/cwd status line both appear in this PR's own fixture — or a bounded latch release after N consecutive frames with no blocking signature. Copy-matching a third-party TUI per release is not a stable contract for a published package.
2. The changeset advertises two behaviours the patch does not implement (.changeset/codex-0143-ready-composer.md:7)
MCP startup warnings remain non-blocking and Claude Code readiness is unchanged.
No code in this diff touches either. The new MCP test (codex.test.ts:252) asserts a string that matches none of the BLOCKING_PROMPT_SIGNATURES phrase pairs, so it is true by construction and could not regress. A changeset is compiled into CHANGELOG.md and shipped in the npm tarball permanently; a consumer reads that line as "this release made MCP warnings non-blocking" or "there was a bug where they blocked". Cut the sentence and describe only the composer fix. (Keep the tests — they are fine as characterisation; they just are not release notes.)
Nits
codex.test.ts:329 — the negative case "does not mistake an onboarding screen for the empty composer" still only covers the 0.147 copy. Add the 0.143 string to it so a future edit dropping the !detectBlockingPrompt(current) guard at session-manager.ts:1179 fails a test.
session-manager.test.ts:707-713 — asserting not-ready at 749 ms then ready at 750 ms pins the settle constant by arithmetic; the neighbouring tests deliberately use a generous window and explain why. Reuse that style so a constant change does not produce a confusing off-by-one failure.
Verdict
Approve after the changeset is trimmed (finding 2). Finding 1 is not a blocker for this release but should get an issue: the current design turns every Codex copy change into a silent prompt-delivery outage for users.
Added a copy-independent composer proof using the › input marker plus the cwd footer, while keeping recognized blocking modals authoritative.
Added coverage for an unknown future placeholder, a selection marker without a footer, and a diff frame containing both the trust modal and underlying composer.
Trimmed the changeset to describe only behavior changed by this patch.
Replaced the exact 749/750 ms boundary assertion with a generous settle-window crossing.
Focused adapter/session-manager tests now pass 143/143; typecheck passes; lint remains at zero errors (one unrelated pre-existing warning).
No confidentiality issues in the new text: the changeset and comments name only Sapiom, Codex and Agent Studio.
New finding
1. The copy-independent proof can release the latch on a partial trust-screen repaint (packages/harness/src/core/adapters/codex.ts:127)
detectBlockingPrompt requires all phrases of a signature in the same frame (codex.ts:322, .every), while the new ready proof needs only › and ·<path> anywhere in that frame — and the two regexes are matched independently, not on the same row. A Ratatui repaint that redraws just the moved selection rows and the footer, e.g.
has no "trust the contents of this directory" line, so detectBlockingPrompt(current) is false and the heuristic is true → blockingPromptSeen clears and readinessHistory is overwritten with that frame (session-manager.ts:1181-1185), so the trust screen can never re-latch. The held initial prompt is then written into the still-open modal. This is exactly the invariant the latch comment asserts at session-manager.ts:1150 ("merely failing to re-render every phrase is not evidence that the screen was dismissed"); the new proof breaks it. The PR's own added test at session-manager.test.ts:698 shows the modal and the composer footer coexisting in one frame — it only passes because that frame happens to contain all three trust phrases.
Fix: require the marker and the footer, plus absence of any single blocking phrase in the frame (a weaker one-phrase veto for the latch-release path only), or require the › to be on a row that is not a numbered selection entry.
Earlier round
Finding 1 (no escape hatch): mitigated, not closed — there is still no bounded latch release; a build whose composer lacks the ·<path> footer hangs indefinitely. Fine to defer to an issue.
Finding 2 (changeset overclaim): fixed. New nit: "recognize future composer copy changes" states as fact what is a two-glyph heuristic — say "best-effort".
Both earlier nits addressed.
Nothing the earlier round got wrong.
Verdict
Request changes on finding 1 — the hardening reintroduces a mis-injection path into the trust prompt.
Addressed the partial-repaint safety finding in 15318ff:
Composer readiness is now vetoed by any single known onboarding fragment. Full blocker detection still requires the complete multi-phrase signature, but an already-latched blocker can only clear on a clean composer frame.
Added the exact partial trust repaint from the review (Yes, continue / selected No, quit plus the underlying cwd footer) at both adapter and session-manager levels.
Updated the changeset to call future-copy recognition best-effort.
Focused adapter/session-manager tests pass 144/144; typecheck passes; lint has zero errors.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Use /skills to list available skillsempty-composer frameRoot cause
The v0.3.3 readiness fallback correctly saw and retained Codex's trust screen, but it only accepted the newer
Ask Codex to do anythingcomposer copy as proof that onboarding had cleared. Codex 0.143 renders a different empty-composer placeholder, so the session remainedready: falseafter trust was accepted and Agent Studio kept holding the initial prompt.The failing Mac's live PTY replay confirmed the trust screen was in history, the current frame contained the 0.143 composer copy, and the MCP warnings did not match a blocking-screen signature.
Validation
The excluded workspace-watcher test was run separately and reproduces unchanged because this sandbox cannot make the fixture directory unreadable with
chmod; it is unrelated to this patch.