fix(codex): write status_line when config.toml has no bare [tui] header - #116
Conversation
_setup_codex() picked its branch from `isinstance(parsed.get("tui"), dict)`,
which tomllib reports as true for a config that only ever declares `tui`
through a subtable such as `[tui.model_availability_nux]` or through top-level
dotted keys. _insert_table_line() then searched the raw text for a `[tui]`
header, found none, and returned the content untouched -- after which
_setup_codex() wrote the unchanged file and printed its success message. The
status line was never installed and is_codex_setup() kept returning False,
with nothing in the output to say so.
_insert_table_line() now handles both shapes: it inserts a `[tui]` table ahead
of the first `[tui.*]` subtable, and for dotted keys it appends
`tui.status_line = [...]` after the last top-level `tui.*` key and before the
first table header, so the value cannot land in the wrong table. Appending a
`[tui]` header in the dotted-key case would have produced "Cannot declare
('tui',) twice" and left the user with an unparseable config, which is worse
than the no-op it replaced.
Every mutating path now runs its candidate through tomllib.loads and falls
back to the original text if it would not parse, so an unanticipated shape --
a quoted dotted key like `tui."my key" = 1`, for one -- degrades to a no-op
rather than corrupting the file. _setup_codex() compares against the original
content and reports failure instead of success when nothing was written.
Verified on Windows against seven config shapes plus this machine's real
~/.codex/config.toml: all produce parseable TOML, six install the status line,
and the quoted-key shape is left byte-identical. Full suite 1525 passed,
23 skipped; ruff and mypy clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f9aecf2838
ℹ️ 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".
| if table is None: | ||
| return content | ||
| return content[: table.end()] + f"\n{line}" + content[table.end() :] | ||
| subtable = re.compile(rf"(?m)^[ \t]*\[{re.escape(name)}\.").search(content) |
There was a problem hiding this comment.
Ignore table-like lines inside multiline strings
When a valid config contains a real dotted tui setting plus a multiline string with a line such as [tui.example], this regex treats the string contents as a subtable and injects the new [tui] block into that string. The resulting document still parses, so validated() accepts it; _setup_codex() then overwrites the user's string and reports success even though the parsed tui.status_line remains absent. Detect the table from TOML syntax rather than raw lines, or at minimum verify the candidate's parsed status line before writing it.
Useful? React with 👍 / 👎.
The dotted-key branch added earlier in this PR writes `tui.status_line` with no `[tui]` header, a shape `_remove_table_line()` and `_replace_table_line()` both miss: `_find_table()` returns None, they hand back the content unchanged, and `_unsetup_codex()` writes the identical file while printing `setup_codex_removed`. Verified on macOS — `is_codex_setup()` stayed True after `usage unsetup`, the same silent no-op this PR exists to fix, moved into uninstall. Both helpers now fall back to the top-level dotted key, validate every candidate through `tomllib.loads` before returning it, and `_unsetup_codex()` compares against the original content on both its restore and remove paths so it cannot report success on an unchanged file. `_setup_codex()`'s no-write path printed `setup_codex_config_unreadable` even though the config had read fine; the new `setup_codex_config_unmodifiable` key names the real cause and covers all three paths. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
追加:反安裝路徑有同一個無聲失敗,本分支已補在 macOS 複核這支 PR 時發現,dotted key 分支寫出的形狀讓
實測輸出(unsetup 後檔案一個位元組都沒變): 就是這支 PR 要修掉的那類無聲失敗,換到了反安裝那邊。本 PR 之前 這次的修法(d8bf3bf)
順帶修正一個訊息
新增 macOS 複核原 PR 的七種形狀只在 Windows 10 驗過,這次在 macOS 重跑 setup → unsetup 完整往返:
每個案例結束後都通過 macOS 完整套件 範圍外,順手記一筆
🤖 Generated with Claude Code |
sync-points 的 tickets.reviewed_pr_through / reviewed_issue_through 早就寫在 docs/UPSTREAM.md 裡,但沒有任何程式讀它們——檢查器只比對分支的 last_reviewed。 那兩個面向不是「查過沒發現」,是根本沒查,而每週的排程報告長得跟查過一樣綠。 艦隊層級問題:24 個 fork 裡 21 個都這樣。 走本檔既有的 _request(urllib + GITHUB_TOKEN),不引 gh:commit 軸一直走 REST, 多一條行為不同的路是負債。workflow 那一步本來就給了 token。 ticket 不做 relevance 分流:commit 軸可以自動略過上游每天的 AI digest,因為那種 雜訊認得出來;但已審視編號以上的每一筆 ticket 都還要人讀。 測試抓到的實作陷阱:GitHub 的 issue 端點會把 PR 一起回傳(共用編號空間),過濾 要用「pull_request 欄位在不在」,不能用真假值——回傳空物件時 PR 會在兩個面向 都被算一次。 驗證:ruff / mypy(201 檔)全過、pytest 1543 passed(唯一失敗是 dev_check 已 記載的本機符號連結權限例外,CI 上不存在);實跑 has_updates=False,報告多出 兩節,分別停在 aqua5230#116 與 aqua5230#92。 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The bug
usage setupprinted✓ Codex status_line 已配置while leaving~/.codex/config.tomlbyte-identical, andis_codex_setup()kept returningFalseafterwards. Nothing in the output said anything had gone wrong._setup_codex()chooses its branch fromisinstance(parsed.get("tui"), dict).tomllibreports that as true for a config that only ever declarestuithrough a subtable:_insert_table_line()then searched the raw text for a[tui]header, found none, and returned the content untouched._setup_codex()wrote the unchanged content and printed success anyway.Not platform-specific —
_setup_codex()through_insert_table_line()has nosys.platformbranching andcodex_home()resolves the same on both. It needs this particular config shape, which is why a machine whose config carries a bare[tui], or notuiat all, never sees it.[tui.model_availability_nux]is written by Codex itself, so the shape arrives on its own.The fix
_insert_table_line()now handles both shapes:[tui.*]subtable → insert a[tui]table ahead of the first onetui.status_line = [...]after the last top-leveltui.*key and before the first table header, so the value cannot land in the wrong tableAppending a
[tui]header in the dotted-key case would raiseCannot declare ('tui',) twiceand leave an unparseable config — worse than the no-op it replaces, which is why that case gets dotted-key treatment rather than a new table.Every mutating path now runs its candidate through
tomllib.loadsand falls back to the original text if it would not parse, so an unanticipated shape degrades to a no-op instead of corrupting the file._setup_codex()compares against the original content and reports failure rather than success when nothing was written.Verification
Seven config shapes plus this machine's real
~/.codex/config.toml, on Windows 10 with Python 3.13.15:[tui.model_availability_nux]subtable only[tui]tabletui.animations = false[features]tuinotfeatures[features]then[tui.*]subtabletui."my key" = 1(quoted dotted key)~/.codex/config.tomlmodel_availability_nuxpreservedAll eight parse as valid TOML afterwards. End-to-end,
usage setupon this machine now leavesis_codex_setup()returningTrue.Full suite 1525 passed, 23 skipped (up from 1519 — six new regression tests);
ruffandmypyclean.🤖 Generated with Claude Code