[WRONG BRANCH] fix(prompt): preserve external TOML escaped paths - #396
[WRONG BRANCH] fix(prompt): preserve external TOML escaped paths#396luvs01 wants to merge 1 commit into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 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 |
![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: 3e28773209
ℹ️ 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".
| return null; | ||
| // A present non-string value or unrecognised spelling fails closed. Only | ||
| // `undefined` above proves that the setting is absent. | ||
| return "<unreadable model_instructions_file>"; |
There was a problem hiding this comment.
When rootValue returns PARSE_FAILED for a Codex-valid document that Bun cannot parse—such as the out-of-safe-range i64 case already documented in this module—and the document has no model_instructions_file, the fallback scan finds nothing and this unconditional sentinel classifies the absent key as external. Consequently, selectBaseVariant rejects every attempt to select a base variant with developer_instructions_not_owned; use scanHasRootKey on parse failure so only an actually present but unreadable assignment fails closed.
AGENTS.md reference: src/AGENTS.md:L10-L10
Useful? React with 👍 / 👎.
| function readModelInstructionsFile(configBytes: string | null): string | null { | ||
| if (configBytes === null) return null; | ||
| const parsed = rootValue(configBytes, "model_instructions_file"); | ||
| if (typeof parsed === "string") return parsed; |
There was a problem hiding this comment.
For externally authored basic strings containing valid \t or \f escapes, this returns Bun's decoded value even though this same module documents that supported Bun versions transpose those escapes relative to Codex's Rust TOML parser. The dashboard therefore reports the wrong path, and computePromptProbeStateFingerprint reads and hashes that wrong file, so editing the real external prompt may leave a cached probe result valid; decode these path literals with Codex-compatible TOML escape semantics rather than trusting Bun's parsed string.
AGENTS.md reference: src/AGENTS.md:L10-L10
Useful? React with 👍 / 👎.
|
✅ Deterministic PR hygiene checks passed. |
⏳ DRAFT
What to do
Its title has been prefixed with |
github-actions
Bot
changed the title
1 participant
Motivation
readModelInstructionsFilepass literals through a narrow decoder (decodeBasicString) which returnsnullfor valid TOML escapes, causing presentmodel_instructions_filekeys to be treated as absent/default.Description
rootValue(configBytes, "model_instructions_file")and return the parsed string when present, making standard TOML escapes (e.g.\uXXXX) survive decoding. (src/codex/prompt-layers.ts)decodeBasicString(m[1]!) ?? m[1]!so that if the restricted decoder refuses an escape we preserve the literal rather than treating it as absent. (src/codex/prompt-layers.ts)model_instructions_fileas an unreadable external value (fail-closed) instead of returningnull. (src/codex/prompt-layers.ts)"\u002Fetc\u002F...") and asserts it is classified asexternal, cannot be silently overwritten by selectingdefault, and that the file remains byte-identical. (tests/codex-prompt-base-variants.test.ts)Testing
bun test tests/codex-prompt-base-variants.test.tswas run and all 15 tests passed.bun run typecheckwas run and succeeded.bun run privacy:scanwas run and succeeded.bun run testwas started as the review-ready gate but encountered existing, unrelated timeouts and HTTP-status expectation failures in other suites; these failures are pre-existing and not introduced by this focused change.Codex Task