Skip to content

fix(server): make Codex aware of its selected model - #13549

Closed
t3dotgg wants to merge 1 commit into
mainfrom
t3code/include-model-identity-in-codex-prompt
Closed

t3dotgg wants to merge 1 commit into
mainfrom
t3code/include-model-identity-in-codex-prompt

Conversation

@t3dotgg

@t3dotgg t3dotgg commented Sep 25, 2026 •

Copy link
Copy Markdown
Member

Codex could store T3 Code's model identity in turn settings without showing it to the model. Catalog-provided collaboration prompts replace the custom instructions where T3 put that context.

Send T3's runtime and tool guidance through per-turn application context instead. Include the catalog model name, selected slug, and reasoning effort, and refresh them when the model changes.

Validated with 132 related tests and a real Codex 0.156.1 regression test against an isolated local Responses endpoint. The regression test failed before the fix and now passes through a model and mode switch. Server typecheck and targeted lint pass.

Created with GPT-6 Astra in Codex.

Summary by CodeRabbit

  • Improvements
    • Runtime context now includes the selected model’s display name and reasoning effort, helping the assistant work with accurate information about the current session.
    • Browser and device guidance is provided according to the tools available, independent of the selected interaction mode.
    • Model information is now available to both text generation and Codex sessions.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Sep 25, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 13.5 KiB 13.5 KiB 0 B (0.0%) 15.1 KiB ✅
Codex Thread snapshot wire 7.1 KiB 7.1 KiB +3 B (+0.0%) 7.3 KiB ✅
Codex Live turn WebSocket wire 6.5 KiB 6.5 KiB −3 B (−0.0%) 7.8 KiB ✅
Codex Live turn WebSocket decoded 56.3 KiB 56.3 KiB 0 B (0.0%) 66.4 KiB ✅
Codex Live turn messages 10 10 0 (0.0%) 21 ✅
Claude Total thread wire 13.5 KiB 13.5 KiB −14 B (−0.1%) 15.1 KiB ✅
Claude Thread snapshot wire 7.1 KiB 7.1 KiB +2 B (+0.0%) 7.3 KiB ✅
Claude Live turn WebSocket wire 6.5 KiB 6.4 KiB −16 B (−0.2%) 7.8 KiB ✅
Claude Live turn WebSocket decoded 57.0 KiB 57.0 KiB 0 B (0.0%) 66.4 KiB ✅
Claude Live turn messages 9 9 0 (0.0%) 21 ✅

Baseline: 568c9bc · PR result: bd37a3f · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 114.0 KiB
  • Claude decoded thread snapshot: 114.7 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

@macroscopeapp

macroscopeapp Bot commented Sep 25, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — The production changes are a focused Codex context fix with targeted tests and no product-default or deployment changes. Human review is required because the new CLI probe test adds an @effect-diagnostics ...:off directive that suppresses a static-analysis diagnostic.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Sep 25, 2026

Copy link
Copy Markdown

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: pingdotgg/t3code/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 49fa0216-a66f-4d96-9785-9e5701781ff9

📥 Commits

Reviewing files that changed from the base of the PR and between 568c9bc and bd37a3f.

📒 Files selected for processing (7)
  • apps/server/src/provider/CodexDeveloperInstructions.ts
  • apps/server/src/provider/Drivers/CodexDriver.ts
  • apps/server/src/provider/Layers/CodexAdapter.ts
  • apps/server/src/provider/Layers/CodexRuntimeInstructionsCliProbe.test.ts
  • apps/server/src/provider/Layers/CodexSessionRuntime.test.ts
  • apps/server/src/provider/Layers/CodexSessionRuntime.ts
  • apps/server/src/provider/RuntimeInstructions.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

Codex collaboration instructions no longer include runtime metadata or browser/device guidance. The session adds those instructions to turn context and can use the provider model catalog to include a model display name.

Changes

Codex runtime instruction context

Layer / File(s) Summary
Separate mode and runtime instructions
apps/server/src/provider/CodexDeveloperInstructions.ts, apps/server/src/provider/RuntimeInstructions.ts
Mode instructions are built separately from runtime instructions. Runtime descriptions can include the model name and slug.
Add runtime context and model catalog
apps/server/src/provider/Layers/CodexSessionRuntime.ts, apps/server/src/provider/Drivers/CodexDriver.ts, apps/server/src/provider/Layers/CodexAdapter.ts
The driver passes snapshot models to the adapter and session runtime. The session adds runtime and browser/device guidance to the t3-code turn context.
Validate runtime instructions
apps/server/src/provider/Layers/CodexSessionRuntime.test.ts, apps/server/src/provider/Layers/CodexRuntimeInstructionsCliProbe.test.ts
Tests check turn context, model metadata, and browser/device guidance. An opt-in CLI probe checks captured request instructions.

Priority: ➖ Normal

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

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant CodexDriver
  participant ManagedProviderSnapshot
  participant CodexAdapter
  participant CodexSessionRuntime
  CodexDriver->>ManagedProviderSnapshot: construct snapshot
  CodexDriver->>CodexAdapter: pass live snapshot models
  CodexAdapter->>CodexSessionRuntime: start session with models
  CodexSessionRuntime->>ManagedProviderSnapshot: resolve model catalog
  CodexSessionRuntime->>CodexSessionRuntime: add runtime instructions to t3-code context
Loading

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to bd37a

No actionable merge-blocking issue is established for this change; it is mergeable after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to bd37a

This changes how the application describes its active configuration and available capabilities to an external service. A metadata field is now included in that guidance, so the trustworthiness of that field matters. Execution restrictions are still applied separately.

Retained concerns

  • Low · security · inferred: A catalog display name is newly interpolated into per-turn runtime guidance. If a lower-trust party can supply an instruction-shaped name, single-line normalization does not keep that text distinct from the surrounding guidance. The available evidence does not establish such a party's access or a successful redirection.
Security review details

Security Blast Radius

  • inferred — The new metadata path can affect turns using a matching model in an instance's provider snapshot. The reviewed changes do not establish a new tenant boundary, credential grant, or network exposure.

Security Findings and Attack Paths

  • inferred — A party able to influence a catalog display name could place instruction-shaped text within the generated runtime description. The necessary attacker access and effective behavioral impact have not been demonstrated.

Trust Boundaries and Controls

  • observed — User turn text is placed in the turn input, not interpolated into the runtime-guidance builder. A named-model lookup requires an exact normalized-slug match, and actual approval and sandbox settings are transmitted separately from the guidance.

Resilience and Maintainability Implications

  • observed — The opt-in downstream probe checks that runtime identity appears in developer-role requests after a model and mode switch. It supports delivery of the new context, but does not prove resistance to conflicting instructions or interruption and retry ordering.

Hardening Proposals

  • proposed — Keep catalog display names visibly delimited as metadata rather than free-form guidance, and verify the permitted catalog-name writers and supported runtime versions before relying on that boundary.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 7 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the primary change: making Codex aware of its selected model.
Description check ✅ Passed The description explains what changed, why the change was needed, and how it was validated. It does not use the template headings or include the checklist, but the required substantive information is …
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.
  • 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

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

@t3dotgg

t3dotgg commented Sep 25, 2026

Copy link
Copy Markdown
Member Author

Closing in favor of #13547, which now covers this PR's scope:

  • The mode prompt stays in the collaboration mode. T3 Code context goes through turn/start.additionalContext in two entries, each under Codex's per-entry token limit.
  • runtime_info includes the display name and the model slug.
  • The context comes back after Codex compacts a thread. Codex drops additionalContext on compaction and does not resend unchanged values, so both PRs lost it before.

Note

Comment by Claude Opus 5.5 in Claude Code, on behalf of @t3dotgg.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant