Skip to content

Pre-prune validity computation breaks Codex open_kitchen and fleet dispatch #4059

Description

@Trecek

Summary

open_kitchen(name="implementation") fails on the first orchestrator message of every Codex-backend session in v0.10.752 with:

Recipe 'implementation' failed structural validation: unknown structural error (stage=recipe_validation, errors=[], suggestions contain only the usual warnings)

The fail-closed guard added by PR #3995 (61802dce4) gates on result["valid"], but load_and_validate computes valid from semantic-rule findings evaluated on the unpruned recipe. With the Codex capability override (backend_supports_git_write="false"), the backend-incompatible-skill rule emits 13 ERROR-severity findings — 9 on steps that are pruned immediately afterward (skip_when_false: inputs.backend_supports_git_write) and 4 on inputs.open_pr-route-guarded steps that the conformance test explicitly exempts via _ROUTE_GUARDED_COMPAT_EXCEPTIONS but compute_recipe_validity does not. So valid=False even though the served (pruned) content is structurally sound (non-empty: 85,728 bytes with defer_unresolved=True / 77,686 with False; zero dangling routes, errors=[]).

This is a latent semantic mismatch dating to PR #3515 (merged Jun 1), which threaded backend_name into validation and silently expanded the meaning of valid from "structurally usable" to "no backend-incompatible step anywhere in the YAML, even guarded ones." No consumer was updated for the expansion:

Backend impact matrix (empirical, installed v0.10.752)

Recipe claude-code codex
implementation valid=True valid=False (13 errors)
remediation valid=True valid=False (13 errors)
implementation-groups valid=True valid=False (13 errors)
merge-prs valid=True valid=False (8 errors)
implement-findings valid=True valid=True
full-audit valid=True valid=True
planner valid=True valid=True
research valid=False (1 error) valid=False (8 errors)

(The research row is a distinct pre-existing recipe defect — tracked separately; see Related work.)

Root cause chain (all empirically confirmed on installed v0.10.752)

  1. CodexBackend declares git_metadata_writable=False (execution/backends/codex.py:521); _backend_capability_overrides yields {"backend_supports_git_write": "false"}, merged last/unoverridable (tools_kitchen.py:560-561 via _promote_capability_keys).
  2. Sequencing defect (the root): in recipe/_api.py (load_and_validate): run_semantic_rules runs at line 397 on the unpruned recipe → _prune_skipped_steps at line 408 → compute_recipe_validity at line 457 consumes the pre-prune findings.
  3. backend-incompatible-skill (rules/rules_backend_compat.py:27-68) has no awareness of skip_when_false guards or route-guard exemptions → 13 ERROR findings for codex.
  4. compute_recipe_validity (registry.py:245-254) folds any ERROR-severity semantic/contract finding into validvalid=False, content non-empty, errors=[].
  5. Rectify: Surface LoadRecipeResult Errors and Fail-Closed on Invalid Content #3995's guard trips on not result.get("valid", False); the envelope renders only the structural errors list → misleading "unknown structural error" (envelope rendering is a separate parallel ticket; see Related work).

Scope of fix (from validated investigation; killed alternatives documented in the appended report)

  1. Compute validity from the post-prune recipe. Move semantic-rule evaluation (or at minimum the validity-relevant findings derivation) after _prune_skipped_steps. The ValidationContext must be rebuilt with the pruned recipe — not reused from the pre-prune build.
  2. Single source of truth for route-guard exemptions. Promote _ROUTE_GUARDED_COMPAT_EXCEPTIONS (currently test-only: tests/recipe/test_backend_reachability.py:31-38) into a production constant consumed by both the validity computation path and the conformance test.
  3. Fleet parity. fleet/_api.py dispatch validation must pass the backend capability overrides (mirroring _promote_capability_keys in the open_kitchen path) so fleet's pruning matches what would actually be served. Note fleet already contains an IL-safe local mirror, _build_capability_overrides (fleet/_api.py:112, currently used only for effective_ingredients at :422, not for validation) — this is a one-line call-site change.
  4. Regression tests through the real producer. Bundled-recipe × backend validity assertions through the real load_and_validate with real capability overrides, asserting valid is True, non-empty content, errors == []. Note: the test added by Rectify: Surface LoadRecipeResult Errors and Fail-Closed on Invalid Content #3995 (test_load_and_validate_produces_valid_content_after_step_pruning) was one assertion short — it never asserts valid.

Implementer cautions (validated):

  • Confirm _api_cache.copy_result errors-list aliasing before shipping (cached findings must reflect post-prune state).
  • Deferred-recall recall path uses defer_unresolved=False vs first-open not _user_overrides_present — both passes must agree on the finding basis or first-open vs recall can disagree on valid.
  • Re-run the 8×2 validity matrix after the change to confirm currently-valid cells stay valid (finding sets shrink post-prune; warning-count test assertions in tests/recipe/ may need updates).
  • Semantic-rule authors should be told rules now see the pruned graph (document in recipe/AGENTS.md).
  • Extend _RECIPE_NAMES in tests/recipe/test_backend_reachability.py:24 (currently implementation / implementation-groups / remediation) to include merge-prs: its 8 codex errors are all on backend_supports_git_write-guarded steps, so the sequencing fix alone makes it valid — but nothing currently guards against regression there.
  • Decide explicitly WHERE the promoted route-guard exemption is consumed: (a) filter findings before compute_recipe_validity (exempted findings stay visible in suggestions), (b) inside the backend-incompatible-skill rule (findings disappear entirely), or (c) inside compute_recipe_validity. The choice changes what callers see in suggestions — pick one and document it.
  • Adjacent consumer with the same defect class (decide: in-scope or follow-up ticket): validate_from_path (recipe/_api_listing.py:71, backing the validate_recipe MCP tool) runs run_semantic_rules (:131) and NEVER calls _prune_skipped_steps, so it reports backend-incompatible false positives on correctly guarded steps whenever a backend name is supplied.

Related work

Parallel split

Filed alongside (no file overlap, parallel-safe):

Investigation

Prior investigation completed interactively. Full root cause analysis below.

Investigation: open_kitchen "unknown structural error" — Codex Backend Total Breakage (v0.10.752)

Date: 2026-06-11
Scope: Regression in open_kitchen(name="implementation") with the Codex backend failing on the first orchestrator message with errors=[] and "unknown structural error"; root cause, test-gap analysis, and process-gap analysis.
Mode: Deep Analysis (2 exploration batches + adversarial challenge round + post-report validation; 9 subagents)

Summary

The failure is caused by the interaction of a latent semantic mismatch (present since PR #3515, merged Jun 1) with a new fail-closed guard (PR #3995 / commit 61802dce4, deployed in v0.10.752 — two commits ahead of the local checkout at 8c2ced093). load_and_validate runs semantic validation rules on the unpruned recipe and folds the resulting findings into the valid flag, so any backend-restricted skill referenced by a step — even one correctly guarded by skip_when_false and pruned moments later — produces an ERROR-severity backend-incompatible-skill finding that makes valid=False for Codex sessions. Before #3995 nothing in open_kitchen consumed valid, so the flag's poisoned value was invisible. #3995 added if not result.get("valid", False) or not result.get("content", "") as a fail-closed gate (intended to stop the silent content-wipe bug from run impl-20260610-095047-486925), which now trips on every Codex open_kitchen for the implementation recipe. The misleading message "unknown structural error" arises because the failure envelope renders only the structural errors list (result["errors"], empty) while the findings that actually drove valid=False are ERROR-severity items inside result["suggestions"]. Empirically confirmed against the installed v0.10.752 package: codex → valid=False (13 ERROR findings, content 78–86KB (defer-flag dependent), errors=[]); claude-code → valid=True for implementation. The same guard also breaks open_kitchen("research") for all backends (1 pre-existing ERROR finding), and the same valid=False mismatch has been silently rejecting Codex fleet dispatch at fleet/_api.py:374 since #3515.

Root Cause

Causal chain (all steps empirically confirmed on the installed v0.10.752 package):

  1. Backend capability override. CodexBackend declares git_metadata_writable=False (execution/backends/codex.py:521); _backend_capability_overrides produces {"backend_supports_git_write": "false"} and open_kitchen merges it last (unoverridable) into ingredient overrides (tools_kitchen.py:561), then calls load_and_validate(..., backend_name="codex") (tools_kitchen.py:648-656).
  2. Sequencing defect (the root). In recipe/_api.py (load_and_validate): semantic rules run at line 397 on the unpruned recipe; _prune_skipped_steps runs at line 408; compute_recipe_validity at line 457 consumes the pre-prune findings. The backend-incompatible-skill rule (rules/rules_backend_compat.py:27-68) has no awareness of skip_when_false guards.
  3. 13 ERROR findings for codex on implementation. 9 findings are on steps guarded by skip_when_false: inputs.backend_supports_git_write (pruned at load time — pure false positives); 4 are on inputs.open_pr-route-guarded steps that the conformance test exempts via _ROUTE_GUARDED_COMPAT_EXCEPTIONS (tests/recipe/test_backend_reachability.py:31-38) but compute_recipe_validity (registry.py:245-254) does not.
  4. Result shape. valid=False, content non-empty (~78–86KB; pruning and route repair work correctly — no dangling routes), errors=[] (no structural errors), 13 error-severity + ~223 warning-severity items in suggestions.
  5. New guard trips. Rectify: Surface LoadRecipeResult Errors and Fail-Closed on Invalid Content #3995's fail-closed check (tools_kitchen.py:711 first open; :624 deferred-recall — the user's second call hit this one) fires on not result.get("valid", False).
  6. Misleading message. _recipe_validation_error_response (tools_kitchen.py:100-115) renders detail only from result.get("errors", []) (structural list); empty → falls back to the literal "unknown structural error". The actual cause (error-severity semantic findings) sits unrendered in suggestions.

Notes:

  • _apply_triage_gate only filters rule=="stale-contract" suggestions (repository.py:139-218); it neither affects valid nor strips the error findings.
  • The Rectify: Surface LoadRecipeResult Errors and Fail-Closed on Invalid Content #3995 commit message explicitly deferred "Part B" (further recipe YAML repairs) — the deployed v0.10.752 was a knowingly intermediate state, but its blast radius (total Codex breakage) was not understood.
  • The visible transcript showed only warning-severity suggestions because the response JSON was truncated in the terminal rendering; the 13 error items are present in the full payload.

Affected Components

  • src/autoskillit/recipe/_api.py (load_and_validate, lines 352-457): rules-before-prune sequencing; validity computed from pre-prune findings [SUPPORTED]
  • src/autoskillit/recipe/registry.py:245-254 (compute_recipe_validity): folds ANY ERROR-severity semantic/contract finding into valid with no prune/route-guard awareness [SUPPORTED]
  • src/autoskillit/recipe/rules/rules_backend_compat.py:27-68 (backend-incompatible-skill): fires on guarded/prunable steps [SUPPORTED]
  • src/autoskillit/server/tools/tools_kitchen.py (v0.10.752: guard at :624 and :711; _recipe_validation_error_response at :100-115): gates on valid; renders only structural errors [SUPPORTED]
  • src/autoskillit/fleet/_api.py:349-394: same valid=False gate (line 374) rejects Codex campaign dispatch (FLEET_RECIPE_INVALID) since Implementation Plan: Thread backend_name Through Recipe Validation API #3515; passes backend_name but no capability overrides, so the internal prune pass runs but removes nothing (the skip_when_false: inputs.backend_supports_git_write guards evaluate against the YAML default 'true') [SUPPORTED]
  • src/autoskillit/recipes/research.yaml: 1 pre-existing ERROR finding (audit-impl-remediation-route) → valid=False for ALL backends → open_kitchen("research") blocked everywhere on v0.10.752 [SUPPORTED]
  • src/autoskillit/recipe/_cmd_rpc_issues.py:293,303: \$ in f-strings → import-time SyntaxWarning (cosmetic; separate defect; see External Research) [SUPPORTED]
  • src/autoskillit/recipe/_api_cache.py (copy_result): cached-result copy does not independently copy the new errors list (aliasing risk only) [NEEDS-EVIDENCE]

Backend impact matrix (empirical, installed v0.10.752):

Recipe claude-code codex
implementation valid=True valid=False (13 errors)
remediation valid=True valid=False (13 errors)
implementation-groups valid=True valid=False (13 errors)
merge-prs valid=True valid=False (8 errors)
implement-findings valid=True valid=True
full-audit valid=True valid=True
planner valid=True valid=True
research valid=False (1 error) valid=False (8 errors)

Data Flow

autoskillit order (env: AUTOSKILLIT_AGENT_BACKEND__BACKEND=codex)
  → open_kitchen(name="implementation")                       [tools_kitchen.py]
    → gate.enable() (line 226 — BEFORE load; explains deferred-recall on 2nd call)
    → _backend_capability_overrides → backend_supports_git_write="false" (merged last, :561)
    → load_and_validate(name, ..., backend_name="codex")       [recipe/_api.py]
        :352 _build_active_recipe          (no skip_when_false evaluation)
        :357 validate_recipe_structure      → errors=[]            (structural OK)
        :397 run_semantic_rules (UNPRUNED)  → 13 ERROR findings    ← false positives
        :408 _prune_skipped_steps           → 9 steps removed, routes repaired OK
        :457 compute_recipe_validity(pre-prune findings) → valid=False
        :519 result = {content: ~78–86KB, errors: [], suggestions: [13 err + ~223 warn], valid: False}
    → guard :711 (1st call) / :624 (2nd call): not valid → trip
    → _recipe_validation_error_response: errors=[] → "unknown structural error"

Test Gap Analysis

Six compounding mechanisms let this reach runtime:

  1. The commit's own test was one assertion short. test_load_and_validate_produces_valid_content_after_step_pruning (added by Rectify: Surface LoadRecipeResult Errors and Fail-Closed on Invalid Content #3995, tests/recipe/test_api.py) calls the REAL load_and_validate with codex + backend_supports_git_write=false — the exact production scenario — but asserts only "no dangling-route errors" and "content non-empty". Adding assert result["valid"] is True would have failed and caught the regression pre-merge.
  2. The conformance test validates the right invariant in the wrong order. test_pruned_recipe_has_no_backend_incompatible_findings (Implementation Plan: Backend-Parametrized Recipe Reachability Tests + Codex Smoke Pipeline Variant #3985, tests/recipe/test_backend_reachability.py:50-75) calls _prune_skipped_steps FIRST, then run_semantic_rules on the pruned recipe — the correct order, which production does not use. It bypasses load_and_validate entirely, so it verifies the desired property while the production code path computes the opposite.
  3. Every open_kitchen-level test hand-mocks load_and_validate. All gating tests (test_tools_kitchen_envelope.py, test_tools_kitchen_visibility.py, test_open_kitchen_deferred_recall.py, test_tools_kitchen_gate.py) construct the result dict by hand. The new Rectify: Surface LoadRecipeResult Errors and Fail-Closed on Invalid Content #3995 envelope tests mock valid=False WITH non-empty errors — the one shape production never produces in this failure (valid=False + errors=[]) is untested.
  4. No end-to-end test crosses the recipe→server boundary. No test exercises bundled YAML → real load_and_validate (with backend capability overrides) → open_kitchen gating decision, for any backend.
  5. CI excludes the only tests that could see it at runtime. .github/workflows/tests.yml runs task test-check, whose Taskfile definition (Taskfile.yml:70) bakes in -m "not smoke and not canary"; the Codex smoke target (task test-smoke-codex) is manual-only, credential-gated, and only validates raw codex exec NDJSON — it never opens a kitchen. PR CI additionally runs path-filtered (AUTOSKILLIT_TEST_FILTER=conservative); the server cascade does not include recipe/ integration tests, so a server-only diff would skip them entirely (in Rectify: Surface LoadRecipeResult Errors and Fail-Closed on Invalid Content #3995's case recipe/_api.py was touched, so recipe/test_api.py DID run — and passed, see gap 1).
  6. Review gates are diff-scoped and text-scoped. dry-walkthrough, audit-impl, and review-pr all ran on Rectify: Surface LoadRecipeResult Errors and Fail-Closed on Invalid Content #3995 (per the commit's pipeline signature). None has an instruction to (a) verify that a newly-consumed key's semantics match the guard's intent when producer logic lives OUTSIDE the diff (compute_recipe_validity and rules_backend_compat.py were not in the diff), or (b) empirically execute the gated code path. The dry-walkthrough Historical Regression Check only catches symbol-level re-additions of previously deleted code, not same-failure-mode recurrence through new symbols.

Similar Patterns

  • Producer/consumer key-contract tests exist but were not applied to this boundary. tests/contracts/test_hook_bridge_coverage.py asserts _quota_guard_hook_payload() produces exactly the keys its consumer reads — the precise pattern missing for LoadRecipeResulttools_kitchen.py. Existing test_load_recipe_result_is_typed checks only TypedDict annotations, not runtime values or semantics.
  • validate_from_path (the validate_recipe MCP tool) already returns {"valid", "errors", "findings", ...}Rectify: Surface LoadRecipeResult Errors and Fail-Closed on Invalid Content #3995's added errors field mirrored that shape, but load_and_validate puts semantic findings in suggestions, not errors, which is exactly the asymmetry the failure envelope tripped over.
  • fleet/_api.py:374 is the older sibling of the same defect: gates dispatch on valid while error detail lives in suggestions (it at least renders error-severity suggestions in its message — the open_kitchen envelope does not).

Design Intent Findings

Historical Context

Part A (project log mining): no prior /investigate logs found in ~/.claude/projects/-home-talon-projects-generic_automation_mcp matching this root cause.

Part B/C (git history): this is the sixth fix in ~14 days in the same "Codex backend × recipe pruning/content delivery" family — and #3995 itself says so in its commit message ("the sixth manifestation of the step-pruning/content-divergence family"):

Commit PR Date Failure mode Added E2E gate?
3c50c8fc0 #3160 May 27 prune left dangling routes for on_result-only steps No
293283d22 #3187 May 28 dataclass vs raw-YAML content divergence No
358c8a168 #3736 Jun 4 Codex read raw YAML, bypassing pruning No
207deda91 #3960 Jun 9 silent close_issue_no_changes when backend can't write No
884e1f2ae #3980 Jun 10 diagram/pruned-content divergence; vacuous guard test No
61802dce4 #3995 Jun 10 silent content wipe → fail-closed guard (this regression) Near-miss (missing valid assertion)

Every fix addressed the layer where the symptom appeared; none added the end-to-end invariant "every bundled recipe × supported backend loads with valid=True and non-empty content through the production path." Additionally, parallel branches diverged during #3995's development (b62f26b81 duplicated a test-isolation fixture; ff18b21e4 — NOT an ancestor of the merged commit — carried overlapping YAML fixes), indicating concurrent uncoordinated work on the same surface.

This is a recurring pattern — consider running /rectify for architectural immunity after resolving the immediate issue.

External Research

No external library involvement; the defect is internal logic. Two empirically verified environment facts (local verification, not web research):

  • The installed tool (uv tool env, Python 3.13.2) at v0.10.752 is byte-identical to origin/develop (dae5f5ded) for all files examined; no local recipe shadowing exists in ~/projects/token-reserve/.autoskillit/recipes/ (directory absent).
  • The SyntaxWarning: invalid escape sequence '\$' at _cmd_rpc_issues.py:293,303 comes from \$ inside f-strings (GraphQL variable syntax cargo-culted from shell escaping — Python needs no escape for $). Cosmetic on Python 3.13 (compile-time warning, emitted once before .pyc caching); Python has long signaled invalid escapes will eventually become hard errors, so it should be fixed (drop the backslashes), but it is unrelated to the regression.

Scope Boundary

Investigated: open_kitchen gating flow (both first-open and deferred-recall paths) in v0.10.752; load_and_validate sequencing; compute_recipe_validity; backend-incompatible-skill rule; backend capability override derivation; all 8 bundled recipes × {claude-code, codex} validity matrix; #3995's full diff, intent, tests, and pipeline gates; fleet dispatch path; CI workflow/smoke coverage; test path filtering; review-skill checklists; 6-commit recurrence history; the \$ SyntaxWarning.

Not yet explored: whether Codex fleet campaigns were ever attempted/observed failing in production telemetry since #3515 (code-level breakage is confirmed; runtime occurrence not mined); the errors-list aliasing in _api_cache.copy_result (flagged, unconfirmed impact); whether other semantic rules besides backend-incompatible-skill and audit-impl-remediation-route can produce ERROR findings that poison valid for specific configurations; the exact fix for the research recipe's audit-impl-remediation-route error.

Recommendations

Single converged recommendation (three coordinated changes — one root fix, one contract fix, one reporting fix):

  1. Root fix — compute validity from the post-prune recipe with shared route-guard exemptions. In load_and_validate, run semantic rules (or at minimum re-derive the validity-relevant findings) on the pruned recipe, and move _ROUTE_GUARDED_COMPAT_EXCEPTIONS out of the test into a production constant consumed by both the backend-incompatible-skill rule path and the conformance test (single source of truth). This makes valid mean what every consumer assumes — "this recipe, as it will actually be served, is usable" — and simultaneously repairs Codex fleet dispatch (fleet/_api.py:374) which has been broken since Implementation Plan: Thread backend_name Through Recipe Validation API #3515. Implementer notes (validated): (a) make_validation_context is currently built from the pre-prune active_recipe — it must be rebuilt with the pruned recipe, not reused; (b) fleet's call must also pass the backend capability overrides (see _promote_capability_keys at tools_kitchen.py:560, which makes them unoverridable in the open_kitchen path) so fleet's pruning matches what would be served; (c) confirm the _api_cache.copy_result errors-list aliasing before shipping, since cached findings must reflect post-prune state; (d) re-run the 8×2 validity matrix after the change to confirm the currently-valid cells stay valid.
  2. Contract fix — encode the envelope invariant. Enforce (by construction or contract test) that whenever valid=False, the failure envelope can name a cause: _recipe_validation_error_response must merge error-severity suggestions into the rendered detail when errors is empty, so "unknown structural error" becomes structurally unreachable. Extend the test_hook_bridge_coverage.py producer/consumer pattern to the LoadRecipeResulttools_kitchen.py boundary.
  3. Test-gap fix — the missing end-to-end invariant. Add a parametrized test (the challenge round's matrix probe is effectively its prototype): for every bundled recipe × supported backend (with that backend's real capability overrides), call the REAL load_and_validate and assert valid is True, content non-empty, errors == []; plus one server-layer test driving real open_kitchen on a bundled recipe. This single test fails today for 5 codex cells and 1 claude-code cell (research), i.e., it catches both the regression and the latent research-recipe defect. Wire at least the claude-code/codex matrix variant into CI (it requires no API credentials — it is pure validation, not a live session).

Also: fix the research recipe's audit-impl-remediation-route ERROR (currently blocks open_kitchen("research") on all backends) — note this needs its own analysis, because change #1 will NOT fix it if the rule fires on a step that survives pruning; and remove the \$ escapes in _cmd_rpc_issues.py:293,303.

Killed alternatives:

Breakage Analysis

  • Recommendation: compute validity from the post-prune recipe with shared route-guard exemptions (change Feature: on_result multi-way routing in YAML pipeline steps #1)
    • Breakage surface: every consumer of suggestions/findings counts — hooks/formatters/_fmt_recipe.py (finding-count rendering), fleet/_api.py:374-388 (rejection messages), warning-count assertions in tests/recipe/ (post-prune evaluation reduces the ~236-finding set), and the deferred-recall path's defer_unresolved=False recall (tools_kitchen.py:583) which may prune differently from the first pass — both passes must use the same finding basis or first-open vs recall can disagree on valid.
    • Prior reverts: none found on _api.py validation ordering (git log --grep="revert" on the mechanism files: no hits); however 6 prior fixes in 14 days on this surface mean any change here needs the new E2E matrix test in the SAME commit.
    • Downstream contract violations: LoadRecipeResult shape is unchanged (no key changes), so formatter/cache contracts hold; semantic-rule authors must know rules now see the pruned graph (document in recipe/AGENTS.md).
    • Risk level: MEDIUM (multi-consumer behavioral shift, mitigated by the matrix test landing first/with it).
  • Recommendation: envelope merges error-severity suggestions (change Feature: read_db MCP tool for read-only SQLite queries #2)
    • Breakage surface: callers parsing user_visible_message strings (none found doing exact-match); _FMT_OPEN_KITCHEN_SUPPRESSED in hooks/formatters/_fmt_recipe.py suppresses errors from agent-visible output and may need the merged detail whitelisted.
    • Prior reverts: none.
    • Downstream contract violations: none — additive message enrichment.
    • Risk level: LOW.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugExisting behavior is brokenrecipe:remediationRoute: investigate/decompose before implementationstagedImplementation staged and waiting for promotion to main

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions