Repository navigation
feat(gui): show the prompt text Codex really sends, as an ordered stack - #2681
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
the
intake: hygiene-blocked
|
✅ READY
Hygiene✅ Deterministic PR hygiene checks passed. |
![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: c076ee7e15
ℹ️ 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".
| join(homedir(), ".codex/packages/standalone/current/bin/codex"), | ||
| join(homedir(), ".local/bin/codex"), | ||
| "/usr/local/bin/codex", | ||
| "/opt/homebrew/bin/codex", | ||
| ]; |
There was a problem hiding this comment.
When Codex is installed only through PATH, CODEX_CLI_PATH, the persisted runtime selection, or a Windows .cmd/.exe launcher, none of these hard-coded Unix paths resolves, so /api/codex-prompt/text always reports codex binary not found even though OpenCodex can launch Codex normally. Resolve the binary through resolveCodexRuntime and launch it with the existing platform-safe codexExecInvocation helper instead of maintaining a second incomplete installation search.
AGENTS.md reference: src/AGENTS.md:L15-L15
Useful? React with 👍 / 👎.
| if (lastIdRef.current !== null) { | ||
| draftsRef.current.set(lastIdRef.current, { title, body }); |
There was a problem hiding this comment.
After a user edits layer A and navigates to an untouched layer B, A's changes are stored only in draftsRef, while dirty compares the visible B fields with B's original values. Pressing Cancel or Escape on B therefore closes immediately without the discard confirmation and silently loses A's parked edits; track dirtiness for every parked draft and include it in requestClose.
Useful? React with 👍 / 👎.
| const res = await fetch(apiBase + "/api/codex-prompt/text"); | ||
| const body = await res.json() as { ok: boolean; layers?: Record<string, { text: string | null; reason: string; bytes: number }> }; | ||
| if (!cancelled) setLayerText(body); |
There was a problem hiding this comment.
When a configuration write completes while a prompt-text request is still pending, invalidation often sets layerText from null to the same null, so this effect is not cleaned up and its cancelled flag remains false. The pre-write probe can then land and publish stale text and byte counts beside the newly updated switches; abort or generation-tag every outstanding probe when invalidating, even when the state is already null.
AGENTS.md reference: gui/AGENTS.md:L9-L10
Useful? React with 👍 / 👎.
| useEffect(() => { | ||
| // Fetch on panel mount, not on first dialog open: size is what a user needs to | ||
| // DECIDE with, and a prompt-budget page that hides which layer costs 15 KB is | ||
| // asking them to guess. |
There was a problem hiding this comment.
This commit adds user-visible prompt-text probing, state-versus-transition grouping, byte counts, and in-dialog custom-layer navigation, but no docs-site/ file is updated and the existing docs contain no prompt-layer guidance. Add documentation describing these controls and the probe's stated limitations so the dashboard behavior is not discoverable only from the UI and implementation comments.
AGENTS.md reference: gui/AGENTS.md:L36-L36
Useful? React with 👍 / 👎.
c076ee7 to
b42fa4d
Compare
github-actions
Bot
removed
the
intake: hygiene-blocked
319943c to
c8de8a3
Compare
리뷰 · 우선순위 54 / 80설명 이 풀 리퀘스트는 레이어 대화 상자에 Codex 가 실제로 보내는 글을 보여 줍니다. 새 모듈 태그 지도는 라이브 출력에서 확인한 것만 둡니다. 초안이 아닙니다. exact-head CI 는 test 네 조각과 gates, react-doctor 가 실패이고 macos 는 진행 중이었습니다. 아래층이 빨간 스택 위에 있어서 이 층만의 신호로 보기 어렵습니다. 프로브는 관리 API 요청마다 외부 바이너리를 띄울 수 있습니다. 시간·바이트 한도는 있지만, 동시 요청이 많으면 프로세스 부담이 됩니다. 캐시나 한 번만 돌리기 규칙은 이 층에 없습니다. PATH 에 있는 src/codex/prompt-text-probe.ts:178 - 프로브는 CODEX_HOME 만 쓰며 호출자 cwd 를 받지 않습니다 메인테이너의 판단이 필요한 지점
너의 추천 지금 합치지 마세요. 아래층 CI 를 먼저 초록으로 만든 뒤 이 층을 다시 맞추세요. cwd 제거와 공백 태그·빈 파일 구분은 유지하세요. 합치기 전에 프로브 동시성이나 짧은 캐시를 한 줄로라도 문서에 적으세요. 이 댓글은 grok-bot이 작성했습니다 |
e95b478 to
c28cd08
Compare
4ca216d to
861a1ce
Compare
… off The dialog claimed Codex "does not expose the assembled text of a built-in layer". That was simply wrong: Codex is open source and ships `codex debug prompt-input`, which renders the model-visible input list as JSON. A new bounded probe reads it and the dialog shows the actual bytes - 10,740 for skills, 15,354 for AGENTS.md on this machine. What is still absent is now named instead of blanket-denied. Three different causes, three different sentences: the base prompt travels outside the printable list, a diff-rendered section can legitimately send nothing on the sampled turn, and the probe itself can fail. Saying which one applies is the difference between a limit and a bug. AGENTS.md needed special handling: it is not wrapped in a tag like the other sections, it arrives as a plain "# AGENTS.md instructions for <path>" block, so tag matching alone reported it as unrendered while its text sat in the same message. The switch moved into the dialog as well. Opening a layer to read it and then having to close the dialog to act on it is a dead end, and the row and the dialog are the same control over the same key. Rows also stop rendering a raw checkbox. They reached for a .switch class that styles a different element; the dashboard's control is .toggle with a knob. Tests follow the markup: aria-checked on a button, not .checked on an input. The probe is lazy - it shells out, so a user who never opens a layer never pays for it - and fail-soft.
This is a prompt-budget page, and it showed fifteen rows of identical visual weight with no indication that Skills costs 10 KB while Environment context costs 278 bytes. Size now sits on the row, so a user deciding what to keep can see what they are paying for instead of opening each layer to find out. That moved the probe from first-dialog-open to panel mount. It costs one bounded spawn on a page the user opened specifically to manage prompts. Density follows the surface: this is an operator list of fifteen rows, not a settings page with three, so row padding drops from 10px to 7px and rows that can actually be acted on carry more type weight than rows that only report. The research lane behind this (Figma, Photoshop, Zapier, Atlassian, W3C APG) agrees on the same point for ordered stacks - order and weight must be visible, not inferred. Test follows the new request shape: the panel now issues two fetches on mount, so the cold-skeleton case holds only the snapshot open and lets the size probe answer immediately. Holding both would have deadlocked rather than measured anything.
…rom a quiet layer The probe took a cwd from the query string, so an authenticated request could read any readable folder's AGENTS.md through the dashboard. It also described whichever directory the caller named rather than the configuration this page claims to show. It now runs in CODEX_HOME and nowhere else. An empty file is not a quiet layer. A 0-byte ~/.codex/AGENTS.md was reported as "sent nothing on the turn we read", which tells a user their layer is idle when the real answer is that the file they wrote is blank. That is a fourth state now, and it names the path so the user can go fill it in. The tag pattern missed <permissions instructions> because it assumed snake_case and that tag has a space in it. The layer had text the whole time and the panel said it sent nothing. Verified against live output rather than the Rust section IDs, which are identifiers and not the rendered tag. Zero sections from a zero-exit probe is a failed read, not fifteen silent layers, so it returns ok:false with a reason. The probe is now bounded in bytes as well as time, settles exactly once, destroys the stream on the way out, and decodes at the end - per-chunk String() corrupts any UTF-8 character that straddles a chunk boundary. Verified both directions by hand: 0 bytes reports empty-source with the path, and 18 bytes of real content reports ok with the text.
… failed Measured text describes the configuration that produced it. Keeping it across a toggle showed a layer's old body and old byte count beside a switch that now reads off, so every write path invalidates it. Five inventory ids had no tag mapping and were simply absent from the probe result. The dialog read that absence as "unavailable", which claims the probe failed when it succeeded. They are listed explicitly as not-exposed now: having no mapping is a smaller and truer claim than having no answer. Two route tests pin the boundaries that were wrong an hour ago: /text takes no caller-supplied directory, and the probe is bounded in bytes as well as in time with a single settlement path and one decode at the end.
….md body Three of the tag mappings were guesses, and guesses in this table are lies to the user: a wrong name reports a layer as having sent nothing while its text sits in the output. I dumped the rendered tags across the feature flags that enable each section and kept only what appeared - skills, apps, plugins, environment, permissions, and the untagged project doc. personality, realtime and collaboration produced no tag under their own flags in this environment, so they join the unsupported list rather than claiming silence. Fifteen layers, and every one of them now says something it can back up. The AGENTS.md capture ran to end-of-message after deleting every tag-shaped block, which swept up unrelated trailing prose AND deleted XML-like text the user had written inside their own file. Codex wraps the body in <INSTRUCTIONS>, so that is the boundary. Verified with a doc containing <angle> brackets: they survive now and the following section stays its own entry. The block is concatenated straight onto the previous closing tag, so the line anchor I first wrote never matched at all.
… state Three asks in one panel. Order was already correct and completely invisible: LAYER_INVENTORY carries it and the panel sorted by it, but a reader had to infer "this is a stack" from vertical position. Rows carry their assembly index now. The index is CANONICAL, gaps included. Lifting the transition notices out leaves 2 and 5 missing from the main list, and that is the point - renumbering each group 1..n would invent an order the runtime does not have. Built-ins get no drag handle for the same reason: their order comes from world_state.rs and a handle would promise a reordering nothing honors. Custom layers are numbered among THEMSELVES rather than continuing the sequence. They concatenate into one developer_instructions section, so sharing a sequence with the built-ins would draw a fifteen-plus-n stack that does not exist. The live-only split is by KIND, not by a scope flag. realtime and model-switch are notices about a change; they have no steady state to describe. Everything else describes configuration or context. Note what the split is NOT: "every turn" versus "sometimes" - sections are diff-rendered, so an unchanged layer of either kind sends nothing. Getting that wrong is what the last two review rounds were about. They keep the locked row shape but not the locked LABEL: "Always on" is false for a notice. It is not on, it fires. Preset switching lands as asked, with the one addition every reference implementation insists on. No shipped product cycles presets with bare arrows; VS Code Profiles, Arc Boosts and Cursor Rules all pair movement with an explicit current item, and arrows without a position lose the user. So prev/next plus "2 / 3". Navigation moves the EDIT TARGET only. Custom layers are not mutually exclusive - every enabled one composes together - so moving between them must never toggle which is active. Unsaved edits are parked per layer while navigating. Blocking navigation whenever the draft is dirty would have avoided the data loss and made the feature pointless: comparing two layers mid-edit is the whole reason to move between them. Driven red by deleting the parking to confirm the test earns its place.
…he keyboard Navigation introduced a state nothing handled: a refresh can remove the layer under an open editor - another tab, a hand edit, a delete on the row behind the dialog. The lookup fell back to null, which turned the editor into a NEW-layer form still holding the deleted text, so Save would have recreated exactly what the user had just removed. The position indicator read "0 / 3" while it happened. Index -1 is now the signal. The editor closes and says why. Silently swapping to a neighbour was the other option and it is worse: it puts someone else's text under a cursor that was mid-sentence. Custom rows reorder with Alt+Arrow as well as the buttons, per the W3C rearrangeable-listbox pattern. The buttons stay - a shortcut nobody discovers is not an affordance - and a bare arrow stays ordinary navigation. Moving the top row up writes nothing rather than burning a revision on an unchanged list. Three contracts the reviewer asked for now have tests: custom layers numbered among themselves rather than continuing the built-in sequence, a save after navigating writing the layer actually on screen with its siblings intact, and the deletion case above. The last was driven red by restoring the null fallback.
.api-code sets white-space: pre, and both it and the dialog rule are single-class selectors - a specificity tie that source order decides. styles.css loads after styles-codex-set.css, so pre won and a 307-byte permissions body rendered as two clipped lines behind a horizontal scrollbar. Reading the layer text is the whole point of the dialog. Chaining the selector with .api-code outranks it outright. The guard asserts on the stylesheet rather than a computed style: happy-dom applies no cascade, so a computed-style assertion passes against the broken rule too. Driven red against the unchained selector before landing.
A 500 body still parses as JSON, and the payload was adopted verbatim, so an error response carrying a `layers` key was reported to the user as measured truth - byte counts and prompt text that were never read. The narrow case matters more than the obvious one: an error with no `layers` already landed in the same "unavailable" branch, so the visible bug only appears when the failing response happens to carry layer-shaped data. The guard was driven red against exactly that case. The test Call body is typed rather than `any`; every assertion already read `body.layers[].id`.
…effect Calling setState synchronously in an effect body cascades an extra render, which is what `bun run lint` rejects here - and it also meant the stale editor was on screen for that intermediate frame. Every input the branch needs is already known while rendering, so it is a derived boolean.
The catalog entries moved down to the layer whose dialog actually calls them, so repeating them here made every locale an object literal with duplicate properties (TS1117) and broke this tip's build. MSGEOF && bun test ./gui/tests/codex-set-stack.test.tsx ./gui/tests/codex-set-custom-layers.test.tsx ./gui/tests/codex-set-prompt-layers.test.tsx ./gui/tests/codex-set-shell.test.tsx ./gui/tests/codex-set-presets.test.tsx ./gui/tests/locale-parity.test.ts ./gui/tests/fr-localization.test.ts 2>&1 | tail -4
9b39484 to
3462ef9
Compare
861a1ce to
cd71a1d
Compare
…realtext feat(gui): show the prompt text Codex really sends, as an ordered stack
…realtext feat(gui): show the prompt text Codex really sends, as an ordered stack
Summary
The dialog used to say Codex "does not expose" a layer's text. That was wrong: Codex is open source and ships
codex debug prompt-input. This layer reads it, so each row shows the bytes it actually costs and the dialog shows the real text rather than a description.The list also becomes an ordered stack — the numbers are the true assembly indices, gaps included, because renumbering each group from one would show an order Codex does not use. Transition notices (
realtime,model-switch) are split out: they announce a change rather than describe state.Stack (merge bottom-up):
/api/codex-promptDepends on #2680. Review this PR's diff only.
Verification
bun test ./tests/codex-prompt-text-probe.test.ts ./tests/codex-prompt-route.test.tsbun test ./gui/tests/codex-set-prompt-layers.test.tsx ./gui/tests/codex-set-stack.test.tsxWhat the probe honestly does not cover, stated in code rather than implied:
base-instructionsis absent —prompt_debug.rsreturnsprompt.inputand discardsbase_instructions.add_sectionregisters state for diffing; it does not emit text, so a section that renders nothing on a first turn is missing from the sample even though its layer exists. That is reported as not rendered, never as an error.permissions instructions, with a space, and a[a-z_]+pattern silently skipped it. Layers with no verified tag say so instead of claiming silence.The last commit fixes a cascade bug found while screenshotting:
.api-codesetswhite-space: preand ties on specificity, so source order won and the body scrolled sideways one clipped line at a time. Its guard was driven red against the unchained selector before landing.Checklist
The probe runs in
CODEX_HOME, never a caller-supplied directory: acwdparameter let an authenticated request read any readable folder's AGENTS.md. It is bounded in both time and bytes, and every failure degrades to unavailable rather than an error page.