fix(codex): emit skills.config as TOML array#89
Conversation
Codex's config parser expects `skills.config` to be a sequence (array),
but the `-c skills.config.N.path=...` dotted-key format created a map
(table), causing "invalid type: map, expected a sequence" on every
Codex session launch.
Switch to a single `-c 'skills.config=[{path = "...", enabled = true}]'`
TOML array literal. Remove the now-unused FRESHELL_CODEX_SKILL_CONFIG_BASE_INDEX
constant from both spawn-spec.ts and terminal-registry.ts.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![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: 40c796ba43
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const tomlEntries = entries.map( | ||
| (entry) => `{path = ${encodeTomlString(entry.path)}, enabled = ${entry.enabled}}` | ||
| ) | ||
| return ['-c', `skills.config=[${tomlEntries.join(', ')}]`] |
There was a problem hiding this comment.
This now emits a single -c skills.config=[...] assignment, which replaces the entire skills.config list instead of adding Freshell-specific entries, so any user-defined Codex skills from ~/.codex/config.toml are dropped for Freshell-launched sessions. The regression only appears for users who already maintain their own skills, but in that case their configured tools silently disappear; the previous indexed override approach avoided clobbering existing entries.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Same error even with index 0. The dotted-key format never worked — not with high indices, not with low indices.
Every Codex session launched through Freshell has been broken since this feature was added. There's no working
additive behavior to regress from.
That said, the reviewer's concern is valid as a feature gap — if a user has custom skills in their config.toml, our
-c skills.config=[...] override would replace them. But that's a pre-existing limitation (the old code had the same
intent but never functioned), not a regression introduced by this PR.
To actually support additivity we'd need to read and parse the user's ~/.codex/config.toml, merge arrays, and emit
the combined result — which would require adding a TOML parser dependency. That feels like a follow-up, not a
blocker for fixing "Codex can't launch at all."
TL;DR: Not a regression. The old code was completely broken. This PR goes from "nothing works" to "works, but
doesn't merge with user-defined skills." Worth noting as a follow-up improvement though.
1 participant
Summary
Error loading config.toml: invalid type: map, expected a sequence in skills.config-c skills.config.N.path=...dotted-key notation creates a TOML map, but Codex expectsskills.configto be a sequence (array)-c 'skills.config=[{path = "...", enabled = true}, ...]'TOML array literal instead.claude/worktrees/to.gitignoreTest plan
expectCodexTurnCompleteArgstest helper to assert TOML array format and reject dotted-key format🤖 Generated with Claude Code