fix(prompt): preserve external TOML escaped paths

[luvs01]

Summary

• model_instructions_file was read by a line scan whose decoder covers only the restricted escape set this module writes. A hand-set key using standard TOML escapes (for example \u002F) failed to decode and was treated as absent, so baseSelection reported default while Codex was in fact using a replaced base prompt - and a later variant selection could silently overwrite a key somebody else set.
• Read the key through the TOML parser first via rootValue, keep the restricted line scan only as a fallback for documents Bun cannot parse, preserve a quoted literal the narrow decoder refuses, and fail closed when the key is present but not a readable string so the selection reports external instead of default.

Verification

• bun test tests/codex-integration/codex-prompt-base-variants.test.ts - 15 pass, including a new regression test for a hand-set key with standard TOML escapes.
• bun test on the four prompt-layer integration test files - 160 pass, 1 platform skip, 0 fail.
• bun x tsc --noEmit - clean.

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.

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

• All CI tests are green on my local testing.

• I pushed my PR to the latest dev commit.

• I resolved all correct Codex and CodeRabbit findings.

• My PR is ready for review.

Summary by CodeRabbit

• Bug Fixes
  • Improved handling of model_instructions_file settings, including paths containing standard TOML escape sequences.
  • Present but unreadable model-instruction settings are now recognized as externally managed instead of being treated as missing.
  • Prevented attempts to overwrite externally managed instructions, while preserving the existing configuration.

[coderabbitai]

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: 4d45de44-763c-440c-a3b6-52549d9a8933

📥 Commits

Reviewing files that changed from the base of the PR and between e64d699 and 137b5f0.

📒 Files selected for processing (3)

• src/codex/prompt-layers.ts
• src/codex/prompt-layers/toml-read.ts
• tests/codex-integration/codex-prompt-base-variants.test.ts

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

---

📝 Walkthrough

Walkthrough

The change exports root-level TOML lookup and updates model instruction file resolution. Present string values remain external selections, undecodable values are preserved, and unreadable values no longer appear absent. Integration coverage verifies selection refusal and byte-for-byte configuration preservation.

Changes

Model instructions resolution

Layer / File(s)	Summary
Root-level TOML lookup src/codex/prompt-layers/toml-read.ts:41	rootValue is now exported. Its parsing, parse-failure handling, and key lookup remain unchanged.
External selection preservation src/codex/prompt-layers.ts:153, 370-388, tests/codex-integration/codex-prompt-base-variants.test.ts:64-76	readModelInstructionsFile first reads the root-level model_instructions_file value. It preserves undecodable literals and returns "<unreadable model_instructions_file>" for present values that cannot be read. The integration test verifies external selection, refusal of default selection with developer_instructions_not_owned, and unchanged configuration bytes.

Priority: ⬇️ Low

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

Change: Bug fix

Suggested reviewers: lidge-j

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
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.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: preserving externally configured paths that contain TOML escape sequences. It matches the implementation and regression test.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[github-actions]

✅ Deterministic PR hygiene checks passed.

[github-actions]

✅ READY

• all PR quality gates passed; the review readiness checklist is complete.

Review readiness checklist

• ✅ All CI tests are green on my local testing.
• ✅ I pushed my PR to the latest dev commit.
• ✅ I resolved all correct Codex and CodeRabbit findings.
• ✅ My PR is ready for review.

✅ 4/4 boxes ticked.

Automatic ready-for-review conversion failed; please mark the pull request ready manually if it is still a draft.
The review-ready label marks this PR as ready; review automation runs independently.
Maintainers notified: @lidge-jun @Ingwannu

[lidge-jun]

리뷰 · 우선순위 70 / 80

이 PR은 config.toml에 사람이 직접 넣은 model_instructions_file 값을 읽는 방식을 고칩니다. 예전에는 줄을 훑고, 이 모듈이 쓰는 좁은 이스케이프만 푸는 decodeBasicString으로만 읽었습니다. \u002F처럼 표준 TOML 이스케이프가 있으면 풀기가 실패하고, 키가 없는 것처럼 보였습니다. 화면의 baseSelection은 default인데 Codex는 이미 다른 파일을 쓰고 있었고, 그 뒤 변형을 고르면 남의 키를 덮어쓸 수 있었습니다.

지금은 먼저 Bun TOML 파서(rootValue)로 읽습니다. 파서가 문서를 못 읽을 때만 예전 줄 스캔을 씁니다. 좁은 디코더가 거절해도 키를 없다고 치지 않고, 따옴표 안의 글자 그대로를 남깁니다. 키가 있는데 읽을 수 있는 문자열이 아니면 "<unreadable model_instructions_file>"을 돌려서 external로 보고, 변형 선택이 덮어쓰지 못하게 합니다. rootValue는 toml-read.ts에서 밖으로 노출됩니다.

베이스는 dev입니다. 이 글을 쓰는 지금 dev 끝은 e64d6994입니다. #5185입니다. 고친 파일은 src/codex/prompt-layers.ts, src/codex/prompt-layers/toml-read.ts, 회귀 테스트 하나뿐입니다. types.ts와 config.ts는 안 바꿉니다. bug와 review-ready 라벨이 있고 준비 체크는 채워져 있습니다. 작성자가 적은 bun test(변형 15통과, 프롬프트 계층 통합 160통과)와 tsc는 여기서 다시 돌리지 않았습니다. 게이트 쪽 CI(enforce-target, hygiene, label, resolve-pr)는 통과입니다.

라인 - src/codex/prompt-layers.ts:384 - Bun이 문서를 못 파싱하고 decodeBasicString도 거절하면, 따옴표가 붙은 원문 그대로를 경로로 씁니다. 소유권은 external로 막히지만, UI나 오류 상세에 보이는 경로는 "…"처럼 이상해질 수 있습니다. 흔한 경우는 Bun이 먼저 풀어서 테스트가 커버합니다.

라인 - src/codex/prompt-layers.ts:388 - 읽을 수 없을 때 쓰는 센티널 문자열 "<unreadable model_instructions_file>"이 external.path로 그대로 나갑니다. 실제 파일 경로와 겹칠 일은 거의 없지만, 사용자에게 이 문구가 보일 수 있습니다.

라인 - tests/codex-integration/codex-prompt-base-variants.test.ts:64 - 회귀는 Bun이 \u002F를 푸는 경로만 검증합니다. 파서가 실패한 뒤 줄 스캔으로 센티널·원문 보존이 되는지는 테스트가 없습니다.

메인테이너의 판단이 필요한 지점

읽을 수 없는 값을 UI에 센티널 문구로 보여줄지, 아니면 external이되 경로 칸은 비우거나 짧은 고정 메시지(unreadable 등)로 둘지 정해 주세요. 덮어쓰기 방지 동작은 어느 쪽이든 같습니다.

너의 추천

머지해도 됩니다. 버그(키를 없다고 착각 → 남의 설정 덮어쓰기)와 고친 방향이 맞고, 회귀 테스트가 핵심 경로를 잡습니다. tip의 #5185와 파일이 안 겹칩니다. types.ts/config.ts 분할과 무관해서 이 PR을 닫지 마세요. 원하면 파서 실패 폴백용 테스트 하나를 더 넣으면 384·388행이 더 안심됩니다. 없어도 머지를 막을 정도는 아닙니다.

이 댓글은 grok-bot이 작성했습니다

[lidge-jun]

추가 리뷰 · 우선순위 69 / 80

이전 리뷰 이후 head가 137b5f03에서 1131bf5b로 바뀌었습니다. 커밋 메시지는 그대로 fix(prompt): preserve external TOML escaped paths 하나이고, 부모만 예전 e64d6994(#5185) 근처에서 지금 0613aaec 쪽으로 옮긴 재베이스입니다. PR이 손댄 세 파일(src/codex/prompt-layers.ts, src/codex/prompt-layers/toml-read.ts, tests/codex-integration/codex-prompt-base-variants.test.ts)은 이전 tip과 바이트 단위로 같습니다. 즉 model_instructions_file을 먼저 Bun TOML(rootValue)로 읽고, 파서가 실패한 때만 줄 스캔으로 넘기며, 좁은 디코더가 거절해도 키를 없다고 치지 않고, 읽을 수 없으면 센티널로 external을 유지해 덮어쓰기를 막는 방향은 그대로입니다. 베이스는 여전히 dev입니다. 지금 dev 끝은 b9483b3b(#5259 문서)이고, 이 브랜치는 그보다 커밋 1개가 뒤입니다. 겹치는 파일은 없습니다. GitHub는 머지 가능으로 보지만 아직 draft이고, review-ready 라벨·게이트 READY(4/4)는 유지됩니다. types.ts/config.ts 분할과는 무관합니다.

라인 - 이번 델타에는 PR 본문 파일 변경이 없습니다. 이전 지적(줄 스캔 폴백에서 따옴표 붙은 원문을 경로로 쓸 수 있음, 센티널 \"<unreadable model_instructions_file>\"이 UI에 보일 수 있음, 파서 실패·센티널 경로 테스트 부재)은 코드가 안 바뀌어 그대로입니다.
라인 - src/codex/prompt-layers.ts:370 - rootValue가 PARSE_FAILED(심볼)를 돌려도 문자열·undefined가 아니라서 줄 스캔으로 넘어갑니다. 의도에 맞고, hasRootKey/rootArrayEntries와 같은 “파서 실패 ≠ 키 없음” 규칙을 따릅니다.
라인 - 게이트는 이 head에서 READY로 다시 찍혔습니다. CodeRabbit 체크는 draft라 skip입니다. hygiene·enforce-target·label·resolve-pr는 통과했습니다.

메인테이너의 판단이 필요한 지점

재베이스만 했으니, 이전과 같이 센티널 문구를 UI에 그대로 둘지·짧은 고정 메시지로 둘지만 정해 주시면 됩니다. 덮어쓰기 방지 동작은 어느 쪽이든 같습니다. tip 1커밋은 문서뿐이라 지금 맞춰 올릴지는 선택입니다.

너의 추천

내용이 안 바뀌었으므로 이전 추천을 유지합니다. 버그 방향과 회귀 테스트(표준 TOML 이스케이프 → external + 덮어쓰기 거부)가 맞고, tip #5259와 파일이 안 겹칩니다. draft면 ready로 올린 뒤 머지해도 됩니다. 이 PR을 닫지 마세요.

이 댓글은 grok-bot이 작성했습니다

[luvs01]

Consolidated into #5540 in native Stack #5505.

Source head: 1131bf5b006af87c243fb18fd0bc4b1d275e36f8. Replacement head: e352aee0343548a606a8d071b3504848c7503a41.

All 1 unique source contribution commit(s) match their carried commits by stable Git patch ID. Original implementation and coverage are preserved. Prepared aggregate checks passed 200 runtime/configuration tests and 20 GUI/locale tests. Combined route/probe rerun passed 119 tests with one POSIX skip; combined GUI tests, typecheck, structure, privacy and ratchet checks passed. Full cross-platform CI, POSIX execution, docs build and independent review remain pending.

Closing this duplicate standalone review entry at the author's request after verifying migration. This is not a merge or release claim; remaining integration checks and reviews are tracked on the draft replacement. Original branches are retained.