refactor(codex): unify top-level allow-list, fixing TOML vs JSON/YAML drift [skip changelog] (#968)

Closes #966. The Codex config top-level allow-list was maintained twice:
`KNOWN_TOP_LEVEL_KEYS` + `KNOWN_TABLE_KEYS` in schemas/codex.rs (TOML
backend) and `KNOWN_CONFIG_TOP_LEVEL_KEYS` in rules/codex.rs (JSON/YAML
backend). They had already drifted, so the same key was accepted by one
backend and flagged by the other:

- `debug` (v0.129 `[debug]` table) and `include_apply_patch_tool`: valid
  as TOML, false-positive `CDX-CFG-006` as JSON/YAML.
- `js_repl_node_path` / `js_repl_node_module_dirs`: valid as JSON/YAML,
  false-positive `CDX-004` as TOML.

Fix: one source of truth. `schemas/codex.rs::is_known_top_level_key`
(over `KNOWN_TOP_LEVEL_KEYS` ∪ `KNOWN_TABLE_KEYS`) is now consulted by
both `detect_unknown_keys` (TOML) and `collect_unknown_codex_keys`
(JSON/YAML). Deleted the duplicate `KNOWN_CONFIG_TOP_LEVEL_KEYS`; added
the two js_repl scalars to the schemas list. Lenient union (102 keys) -
strictly fewer false positives, both backends now agree.

Incidentally removes the duplication gemini-code-assist flagged on #964.
Nested allow-lists (features/tui/mcp_servers/shell_env/permissions/apps)
already live only on the rules side and are untouched.

Regression tests: `test_codex_unified_allowlist_fixes_json_toml_drift`
(JSON accepts `debug`) and `test_codex_unified_allowlist_fixes_toml_drift`
(TOML accepts `js_repl_node_path`).

Author: avifenesh

crates/agnix-core/src/rules/codex.rs

@@ -132,115 +132,6 @@ const KNOWN_PERMISSIONS_NETWORK_KEYS: &[&str] = &[
 
 const VALID_WINDOWS_SANDBOX_VALUES: &[&str] = &["elevated", "unelevated"];
 
-// KNOWN_CONFIG_TOP_LEVEL_KEYS is the parallel allow-list used for .codex/config.{json,yaml}
-// unknown-top-level-key detection. Keep it in sync with KNOWN_TOP_LEVEL_KEYS ∪ KNOWN_TABLE_KEYS
-// in schemas/codex.rs (this set represents the flat union - the JSON/YAML backends don't
-// distinguish scalar top-level keys from TOML `[section]` tables).
-// Sourced from upstream codex-rs/core/config.schema.json; last reconciled @ rust-v0.133.0. Alphabetized.
-const KNOWN_CONFIG_TOP_LEVEL_KEYS: &[&str] = &[
-    "agents",
-    "allow_login_shell",
-    "analytics",
-    "approval_policy",
-    "approvals_reviewer",
-    "apps",
-    "apps_mcp_product_sku",
-    "audio",
-    "auto_review",
-    "background_terminal_max_timeout",
-    "chatgpt_base_url",
-    "check_for_update_on_startup",
-    "cli_auth_credentials_store",
-    "commit_attribution",
-    "compact_prompt",
-    "default_permissions",
-    "desktop",
-    "developer_instructions",
-    "disable_paste_burst",
-    "experimental_compact_prompt_file",
-    "experimental_realtime_start_instructions",
-    "experimental_realtime_ws_backend_prompt",
-    "experimental_realtime_ws_base_url",
-    "experimental_realtime_ws_model",
-    "experimental_realtime_ws_startup_context",
-    "experimental_thread_config_endpoint",
-    "experimental_thread_store",
-    "experimental_thread_store_endpoint",
-    "experimental_use_freeform_apply_patch",
-    "experimental_use_unified_exec_tool",
-    "features",
-    "feedback",
-    "file_opener",
-    "forced_chatgpt_workspace_id",
-    "forced_login_method",
-    "ghost_snapshot",
-    "hide_agent_reasoning",
-    "history",
-    "hooks",
-    "include_apps_instructions",
-    "include_collaboration_mode_instructions",
-    "include_environment_context",
-    "include_permissions_instructions",
-    "instructions",
-    "js_repl_node_module_dirs",
-    "js_repl_node_path",
-    "log_dir",
-    "marketplaces",
-    "mcp_oauth_callback_port",
-    "mcp_oauth_callback_url",
-    "mcp_oauth_credentials_store",
-    "mcp_servers",
-    "memories",
-    "model",
-    "model_auto_compact_token_limit",
-    "model_auto_compact_token_limit_scope",
-    "model_catalog_json",
-    "model_context_window",
-    "model_instructions_file",
-    "model_provider",
-    "model_providers",
-    "model_reasoning_effort",
-    "model_reasoning_summary",
-    "model_supports_reasoning_summaries",
-    "model_verbosity",
-    "notice",
-    "notify",
-    "openai_base_url",
-    "oss_provider",
-    "otel",
-    "permissions",
-    "personality",
-    "plan_mode_reasoning_effort",
-    "plugins",
-    "profile",
-    "profiles",
-    "project_doc_fallback_filenames",
-    "project_doc_max_bytes",
-    "project_root_markers",
-    "projects",
-    "realtime",
-    "review_model",
-    "sandbox_mode",
-    "sandbox_workspace_write",
-    "service_tier",
-    "shell_environment_policy",
-    "show_raw_agent_reasoning",
-    "skills",
-    "sqlite_home",
-    "suppress_unstable_features_warning",
-    "tool_output_token_limit",
-    "tool_suggest",
-    "tools",
-    "tui",
-    "web_search",
-    "windows",
-    "windows_wsl_setup_acknowledged",
-    "zsh_path",
-    // Legacy compatibility in existing fixtures/tests.
-    "approvalMode",
-    "fullAutoErrorMode",
-];
-
 const KNOWN_FEATURE_KEYS: &[&str] = &[
     "apply_patch_freeform",
     "apps",
@@ -2062,7 +1953,9 @@ fn collect_unknown_codex_keys(root: &Value, cdx_cfg_028_active: bool) -> Vec<Str
     };
 
     for key in root_obj.keys() {
-        if !KNOWN_CONFIG_TOP_LEVEL_KEYS.contains(&key.as_str()) {
+        // Single source of truth lives in schemas/codex.rs so the TOML and
+        // JSON/YAML backends can never drift on the top-level allow-list.
+        if !crate::schemas::codex::is_known_top_level_key(key.as_str()) {
             unknown.push(key.clone());
         }
     }
@@ -3403,10 +3296,11 @@ experimental_thread_store_endpoint = "https://thread-store.example"
     fn test_codex_v0_123_top_level_keys_accepted_json() {
         // JSON/YAML path: CDX-CFG-006 fires for unknown top-level keys when
         // the file is JSON/YAML (NOT TOML, where CDX-004 takes over via
-        // skip_top_level). The fix in rules/codex.rs::KNOWN_CONFIG_TOP_LEVEL_KEYS
-        // must include `experimental_thread_store_endpoint` for CDX-CFG-006
-        // to accept it. Without that arm of the fix, this test fails even if
-        // the TOML schema allow-list (schemas/codex.rs) is updated.
+        // skip_top_level). The shared allow-list in
+        // schemas/codex.rs::is_known_top_level_key must include
+        // `experimental_thread_store_endpoint` for CDX-CFG-006 to accept it.
+        // Both backends consult the same predicate, so TOML and JSON/YAML
+        // can no longer disagree about it.
         let json = r#"{
   "experimental_thread_store_endpoint": "https://thread-store.example"
 }"#;
@@ -3750,11 +3644,11 @@ hide_full_access_warning = true
 
     #[test]
     fn test_codex_0_128_0_new_json_yaml_keys_not_flagged() {
-        // Regression guard for the JSON/YAML unknown-top-level-key path
-        // (KNOWN_CONFIG_TOP_LEVEL_KEYS). Mirror of the TOML-side tests in
-        // schemas/codex.rs; the two allow-lists are deliberately separate
-        // because the JSON/YAML path treats top-level scalars and objects
-        // uniformly while the TOML path splits scalar vs `[table]`.
+        // Regression guard for the JSON/YAML unknown-top-level-key path,
+        // which now consults schemas/codex.rs::is_known_top_level_key (the
+        // shared predicate over KNOWN_TOP_LEVEL_KEYS ∪ KNOWN_TABLE_KEYS).
+        // Mirror of the TOML-side tests in schemas/codex.rs; the JSON/YAML
+        // path treats top-level scalars and `[table]` keys uniformly.
         let json = r#"{
             "auto_review": {},
             "experimental_thread_store": {},
@@ -3783,6 +3677,27 @@ hide_full_access_warning = true
         );
     }
 
+    #[test]
+    fn test_codex_unified_allowlist_fixes_json_toml_drift() {
+        // Drift fix (issue #966): before unifying the top-level allow-list, the
+        // JSON/YAML path flagged `debug` (a valid TOML `[debug]` table from the
+        // v0.129 catch-up) because it lived only in the TOML schema list.
+        // Both backends now share schemas/codex.rs::is_known_top_level_key, so
+        // `debug` is accepted as JSON/YAML too.
+        let json = r#"{ "debug": {}, "include_apply_patch_tool": true }"#;
+        let diags = validate_config_at_path(".codex/config.json", json);
+        assert!(
+            diags
+                .iter()
+                .all(|d| d.rule != "CDX-004" && d.rule != "CDX-CFG-006"),
+            "unified keys should not be flagged on the JSON path, got: {:?}",
+            diags
+                .iter()
+                .filter(|d| d.rule == "CDX-004" || d.rule == "CDX-CFG-006")
+                .collect::<Vec<_>>()
+        );
+    }
+
     #[test]
     fn test_cdx_000_reports_json_yaml_parse_errors() {
         let invalid_json = r#"{"approval_policy":"always""#;

crates/agnix-core/src/schemas/codex.rs

@@ -132,6 +132,10 @@ pub const KNOWN_TOP_LEVEL_KEYS: &[&str] = &[
     "apps_mcp_product_sku",
     "include_collaboration_mode_instructions",
     "model_auto_compact_token_limit_scope",
+    // Unified from the former rules/codex.rs JSON/YAML allow-list (js_repl
+    // node-resolution scalars) so the TOML and JSON/YAML backends agree.
+    "js_repl_node_module_dirs",
+    "js_repl_node_path",
     // Legacy camelCase keys (backwards compat)
     "approvalMode",
     "fullAutoErrorMode",
@@ -184,6 +188,18 @@ pub const KNOWN_TABLE_KEYS: &[&str] = &[
     "desktop",
 ];
 
+/// Single source of truth for whether a Codex config top-level key is known.
+///
+/// Both backends consult this: the TOML path (`detect_unknown_keys` here) and
+/// the JSON/YAML path (`rules::codex::collect_unknown_codex_keys`). Keeping one
+/// predicate prevents the two from drifting (a key valid as TOML but flagged as
+/// JSON, or vice versa). A top-level key may be either a scalar
+/// (`KNOWN_TOP_LEVEL_KEYS`) or a `[section]` table (`KNOWN_TABLE_KEYS`).
+#[must_use]
+pub fn is_known_top_level_key(key: &str) -> bool {
+    KNOWN_TOP_LEVEL_KEYS.contains(&key) || KNOWN_TABLE_KEYS.contains(&key)
+}
+
 /// An unknown key found in config
 #[derive(Debug, Clone)]
 pub struct UnknownKey {
@@ -396,9 +412,7 @@ fn detect_unknown_keys(
     // and this avoids HashSet allocation on every call.
     let mut unknown = Vec::new();
     for key in table.keys() {
-        if !KNOWN_TOP_LEVEL_KEYS.contains(&key.as_str())
-            && !KNOWN_TABLE_KEYS.contains(&key.as_str())
-        {
+        if !is_known_top_level_key(key.as_str()) {
             unknown.push(UnknownKey {
                 key: key.clone(),
                 line: find_toml_key_line(content, key).unwrap_or(1),
@@ -741,6 +755,28 @@ nested_number = 42
         );
     }
 
+    #[test]
+    fn test_codex_unified_allowlist_fixes_toml_drift() {
+        // Drift fix (issue #966): before unifying the top-level allow-list, the
+        // TOML path flagged `js_repl_node_path` / `js_repl_node_module_dirs`
+        // because they lived only in the JSON/YAML list on the rules side.
+        // They are now part of KNOWN_TOP_LEVEL_KEYS, so the TOML path accepts
+        // them too.
+        let content =
+            "js_repl_node_path = \"/usr/bin/node\"\njs_repl_node_module_dirs = [\"/x\"]\n";
+        let result = parse_codex_toml(content);
+        assert!(result.parse_error.is_none());
+        assert!(
+            result.unknown_keys.is_empty(),
+            "js_repl_* keys should not be flagged on the TOML path, got: {:?}",
+            result
+                .unknown_keys
+                .iter()
+                .map(|u| u.key.as_str())
+                .collect::<Vec<_>>()
+        );
+    }
+
     #[test]
     fn test_unknown_keys_empty_on_parse_error() {
         let content = "invalid = [unclosed";