Skip to content

fix(responses): give routed models the Codex App visualization directive in ASCII - #6045

Merged
lidge-jun merged 3 commits into
devfrom
codex/directive-marker-bridge
Sep 27, 2026
Merged

lidge-jun merged 3 commits into
devfrom
codex/directive-marker-bridge

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Sep 27, 2026 •

Copy link
Copy Markdown
Owner

Summary

With a Claude-routed model, the Codex App Visualize plugin printed visualize{"path":…} as plain text instead of drawing the visualization. The plugin asks the model to reply with U+E200 visualize U+E202 {json} U+E201, and every Claude route we checked (direct Anthropic and through Cursor) drops those private-use characters before the model sees them. The model reads the template without its markers and writes it back the same way. A code-point echo test shows the difference: GPT and Grok return all six characters of [A U+E200 B U+E202 C U+E201], while Claude returns only A B C. OpenCodex's own Anthropic request body still contains them.

The app does not need the private-use form. Its renderer (app 26.924.22138) rewrites each span into the plain directive ::codex-inline-vis{path="…"} before parsing, and it renders that directive when it appears directly. This PR applies the same rewrite to the conversation text that routed models read:

  • src/responses/visualization-directives.ts converts each U+E200visualize U+E202…U+E201 span into ::codex-inline-vis{…} (or ::codex-live-vis{…}), using the app's own payload rules. Those rules cover JSON versus bare path, the .html basename, .. and quote rejection, absolute-path detection including Windows and UNC, the path/file attribute, title, and wide mode. A span the app would reject is left exactly as written. The scan matches the app's regex in linear time.
  • Two differences from the app are intentional. Fenced code is rewritten too, because the plugin's example sits in a fence. The plugin's <absolute-path>/<title>.html placeholder is accepted, so the model still sees the template.
  • parseRequest applies the rewrite to the context it returns, so every context-built adapter gets it. _rawBody is untouched, which keeps native passthrough byte-identical and stored previous_response_id history unchanged.

The citation filter from #6040 is unchanged. Replies already saved in the bare form are not repaired.

Verification

  • bun test tests/responses/visualization-directives.test.ts: 18 pass. The three parser and Anthropic integration cases fail with the parser.ts hook removed.
  • The new tests plus the citation, bridge, parser and state suites, and the Lab boundary, file-size ratchet and test-layout guards: 461 pass across 12 files.
  • bun run typecheck, bun run structure:check, and bun run privacy:scan pass.
  • An independent review compared toAsciiDirective and the scanner against a port of the app's f2 over 132,715 generated cases and found no mismatch outside the two documented differences. It also checked that frozen inputs are not mutated and that _rawBody stays identical.
  • bun test tests/responses tests/adapters locally: 5885 pass and 134 fail. Every failing file either passes when run alone or fails identically with the parser.ts hook removed, because this checkout sits under ~/.codex and cross-file state leaks there. The full suite is left to CI.
  • The live Codex App render with a Claude-routed model will be confirmed after the local app is rebuilt from dev. Computer Use cannot inspect the Codex app, so a person has to confirm it.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Summary by CodeRabbit

  • New Features
    • Visualization references in routed-model conversation text are converted into Codex App inline directives, including live visualizations. Native OpenAI passthrough remains unchanged.
  • Documentation
    • Added guidance on inline visualizations and clarified that previously saved bare visualization replies are not converted; request them again to generate a visualization.

@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner September 27, 2026 03:20
@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-27T03:23:11.752226Z 44d564b PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions github-actions Bot added the bug Something isn't working label Sep 27, 2026
@coderabbitai

coderabbitai Bot commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 2ffdcf97-f77b-422b-b13b-7a3d718c90ac

📥 Commits

Reviewing files that changed from the base of the PR and between a1285fc and 44d564b.

📒 Files selected for processing (11)
  • devlog/_plan/260927_directive_marker_bridge/000_plan.md
  • devlog/_plan/260927_directive_marker_bridge/001_app_and_external_evidence.md
  • devlog/_plan/260927_directive_marker_bridge/010_wp2_visualization_directive_normalization.md
  • devlog/_plan/260927_directive_marker_bridge/020_wp3_rebuild_and_live_verify.md
  • docs-site/src/content/docs/guides/codex-integration.md
  • scripts/test-layout/layout.json
  • src/responses/parser.ts
  • src/responses/visualization-directives.ts
  • structure/transports/responses-wire-shapes.md
  • tests/fixtures/test-layout-expected.json
  • tests/responses/visualization-directives.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The parser now converts valid private-use visualization references in model-visible conversation text into Codex App directives. Raw request bodies, stored continuation history, and non-text content remain unchanged. Tests and documentation describe the conversion rules and request-parsing scope.

Changes

Visualization Directive Normalization

Layer / File(s) Summary
Define and implement directive normalization
src/responses/visualization-directives.ts, tests/responses/visualization-directives.test.ts, devlog/_plan/260927_directive_marker_bridge/*
The normalizer validates payloads and paths before converting valid spans to inline or live directives. Tests cover conversion rules, invalid and incomplete references, matching edge cases, and idempotence. The planning notes record the parser-side scope and implementation decisions.
Apply normalization to parsed requests
src/responses/parser.ts, tests/responses/visualization-directives.test.ts, scripts/test-layout/layout.json, tests/fixtures/test-layout-expected.json, structure/transports/responses-wire-shapes.md, docs-site/src/content/docs/guides/codex-integration.md, devlog/_plan/260927_directive_marker_bridge/020_wp3_rebuild_and_live_verify.md
parseRequest normalizes the assembled context. Tests check normalized conversation text and replayed summaries, while confirming preservation of raw bodies, images, tool-call arguments, and unchanged context identity. The docs describe routed-model behavior and unchanged native passthrough; the verification plan records rebuild and live-render checks, with rendering still pending confirmation.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant Parser as parseRequest
  participant Normalizer as normalizeVisualizationContext
  participant Adapter as Anthropic adapter
  Parser->>Normalizer: assembled conversation context
  Normalizer-->>Parser: context with valid directives normalized
  Parser->>Adapter: parsed request context
  Adapter-->>Adapter: serialize message text
Loading

Merge Risk: ⚪ Minimal · up to 44d56

No concrete issue currently blocks merging. Confirm live rendering as planned after rebuilding the app.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 44d56

The change affects what routed models see, but the reviewed code preserves the original request and validates references before rewriting them. No introduced security vulnerability was established. The app’s rendering behavior was not independently verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • observed — Text from any parsed message role can be rewritten before provider submission. The helper itself neither opens the referenced path nor invokes a tool.

Trust Boundaries and Controls

  • inferred — The changed boundary is untrusted conversation text becoming model-visible directive syntax, not a demonstrated new file-access authority. Direct ASCII input is described as already renderable, but the renderer was not available for an independent base-versus-head check.

Resilience and Maintainability Implications

  • observed — Non-text parts and tool-call arguments are outside the normalizer’s rewrite path; the parser also keeps the raw body separate. Durable-history behavior was not established by the inspected source.

Hardening Proposals

  • proposed — Verify at the app renderer that resolving an accepted absolute path remains subject to the intended file-access policy; lexical validation in this helper is not an authorization check.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 55.56% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 3 files. (8 skipped: 8… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: converting visualization markers for routed models into the Codex App's ASCII directive.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 55.56% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 3 files. (8 skipped: 8 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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.

❤️ Share

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

@lidge-jun
lidge-jun merged commit dc784d3 into dev Sep 27, 2026
33 of 34 checks passed
@lidge-jun
lidge-jun deleted the codex/directive-marker-bridge branch September 27, 2026 03:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant