Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughCatalog synchronization now compares generated catalog bytes with the existing file before writing. Unchanged syncs preserve the file modification time and report ChangesCatalog synchronization
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
✅ Deterministic PR hygiene checks passed. |
✅ READY
Review readiness checklist
✅ 4/4 boxes ticked. This pull request is already Ready for Review. |
Ingwannu
left a comment
There was a problem hiding this comment.
The reported issue is real, and this is the right minimal direction: avoiding a true no-op write keeps the catalog mtime meaningful without weakening the existing permit, revalidation, or atomic-write path. I reproduced the focused suite on exact PR head 7bf8b84a after installing the locked dependencies: 100 pass / 0 fail; typecheck and privacy scan also pass.
I am requesting one correctness change before this becomes review-ready:
currentCatalogFileContent() in src/codex/catalog/sync.ts claims to compare exact bytes, but readFileSync(path, "utf8") === content compares decoded JavaScript strings. Invalid UTF-8 bytes are replaced with U+FFFD, so different on-disk bytes can compare equal to a legitimately encoded replacement character. I reproduced the boundary directly: a buffer containing raw byte 0x80 and a buffer containing UTF-8 EF BF BD have equal decoded strings but unequal bytes. In a catalog string field, this path can therefore preserve malformed bytes, skip the atomic repair write, and return catalogWritten: false even though the prepared bytes are not identical to the file.
Please read the file as bytes and compare it with the UTF-8 bytes of content (Buffer.equals or an equivalent exact Uint8Array comparison), while preserving the current unreadable/absent fallback to a real write. Add a regression where an invalid on-disk byte decodes to the same string as a real U+FFFD in the prepared content and assert that the sync still rewrites it. The existing mtime no-op regression should continue to prove that genuinely identical bytes are skipped.
The branch is currently four commits behind dev, still inside the repository's accepted freshness window. Once the exact-byte boundary is fixed, the focused checks are rerun, and the four readiness boxes are completed against the resulting head, this remains a strong merge candidate for #1459.
7bf8b84 to
0b0fee6
Compare
|
@Ingwannu thanks — the finding is correct and is fixed on new head
Regression added: Re-verified on The branch was force-pushed to rewrite the commit author, so the SHA you reviewed is superseded. Sorry for the extra round. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
…ark Codex stale `writeRetainedCatalogSync` wrote the catalog unconditionally, with no content comparison, so every sync moved the file's mtime even when the produced bytes were identical to what was already on disk. `collectCodexAppServerCatalogState()` compares that mtime against each running Codex's start time, so an ordinary `ocx start` — or any dashboard action that re-syncs an unchanged model set — classified every already-running Codex as holding an outdated in-memory catalog. Since lidge-jun#1407 that verdict withholds all opencodex-authored v2 model guidance, so a configured `injectionModel` and roster silently stop reaching sessions for the rest of that Codex's lifetime, even though the advertised model set never changed. A long-lived Codex App app-server outlives every proxy restart, which makes the state effectively latched: restarting the CLI does not clear it. An identical write is now skipped and reported as `catalogWritten: false`. `added` still reports the routed rows the catalog carries, because they are on disk either way, and `cacheSynced` is unaffected (`refreshCodexModelCatalog` invalidates the models cache whenever the catalog exists, independently of whether it was rewritten). The comparison is byte-exact, not a decoded-string comparison. Reading the file as UTF-8 replaces every invalid sequence with U+FFFD, so a malformed byte on disk would compare equal to a legitimately encoded replacement character in the prepared content: the skip would then preserve the corruption and report `catalogWritten: false` for a file that is not what we prepared. An unreadable or absent file still falls back to a real write. The convergence/retained-sync byte-agreement assertions asserted `catalogWritten: true` for exactly the no-op case while also asserting the bytes did not change; they now assert the no-op they describe (no policy skip, no write, bytes unchanged). Assertions covering syncs that really change content are untouched.
0b0fee6 to
642805c
Compare
|
Rebased onto the current Verified locally on
Scope note on the full-suite claim, so the attestation is precise: the last full |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== PR head and changed files ==='
git rev-parse HEAD
git diff --name-status HEAD^1 HEAD
echo '=== Diff ==='
git diff --find-renames --unified=80 HEAD^1 HEAD -- \
src/codex/catalog/sync.ts \
tests/codex-catalog-sync-hardening.test.ts \
tests/codex-convergence-account-selectors.test.ts \
docs-site/src/content/docs/guides/sub-agent-surface.md
echo '=== sync.ts outline ==='
ast-grep outline src/codex/catalog/sync.ts --items all --view expanded
echo '=== Catalog write/read references ==='
rg -n -C 4 -P '\b(currentCatalogFileBytes|currentCatalogFileContent|catalogWritten|writeFile|readFile|withPermit|permit)\b' \
src/codex/catalog/sync.ts tests/codex-catalog-sync-hardening.test.ts tests/codex-convergence-account-selectors.test.tsLength of output: 78987 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== syncCatalogModels commit path ==='
sed -n '1450,1520p' src/codex/catalog/sync.ts
echo '=== Catalog replacement primitive definitions and callers ==='
rg -n -C 8 -P '\b(function|const)\s+(replaceActiveCodexCatalog|withCatalogWriteSerialization)\b|\breplaceActiveCodexCatalog\s*\(|\bwithCatalogWriteSerialization\s*\(' src
echo '=== catalogWritten consumers ==='
rg -n -C 8 -P '\bcatalogWritten\b' --glob '!tests/**' .
echo '=== Refresh and cache synchronization entry points ==='
rg -n -C 6 -P '\b(syncCatalogModels|cacheSynced|invalidateCodexModelsCache|refreshCodexModelCatalog)\s*\(' srcLength of output: 80627
I found no blocking issues. The update in The tests cover unchanged catalog bytes, real catalog changes, malformed UTF-8 with replacement-decoding equivalence, and convergence output. You are interacting with an AI system. |
|
Tip For best results, initiate chat on the files or code changes.
You are interacting with an AI system. |
|
Landed on Your fix is on You independently reached the same Regression on Thank you — this was a good catch and a clean fix. |
…ot claim disk state changed
`invalidateCodexModelsCacheWithPermit` rewrote Codex's models cache
unconditionally, with no comparison against what was already on disk, and
returned `true` for having done so. `refreshCodexModelCatalog` reports that
boolean as `cacheSynced`, and `handleStart` ORs it into the stale-app-server
warning:
if (consumeStartupCacheInvalidationWrite() || startupSync.catalogWritten || startupSync.cacheSynced) {
warnIfStaleCodexAppServersAfterStartupWrite({ log: console });
}
So on a start where the catalog reproduced byte-identically — the settled case —
the warning asserted "Disk catalog/cache were updated" and told the operator
their Codex model list might be stale, for a write that never happened.
`writeRetainedCatalogSync` already skips a byte-identical catalog, so
`catalogWritten` is honest; `cacheSynced` was not, and it alone is enough to
raise the warning. That is the exact failure mode the catalog half of this rule
was added to stop.
This is the second writer that lidge-jun#1459 left uncovered. That issue fixed
`writeRetainedCatalogSync` with a byte-exact no-op guard, and its commit recorded
the independence deliberately — "cacheSynced is unaffected
(refreshCodexModelCatalog invalidates the models cache whenever the catalog
exists, independently of whether it was rewritten)". That is accurate: the
invalidation is attempted whenever the catalog exists, which is precisely why it
could report a write on a start that changed nothing.
The rule now lives once, in `preparedBytesDifferFromDisk`, and both writers
apply it. It stays a Buffer comparison rather than a decoded string for the
reason lidge-jun#1460 established: `readFileSync(path, "utf8")` substitutes U+FFFD for
every invalid byte, so a malformed file would decode equal to prepared content
holding a real U+FFFD, and the guard would preserve corruption while skipping
the atomic repair. An unreadable or absent file reports "differs" so the caller
performs the real write.
`pullRemoteCatalog` already guards this exact pair with a byte comparison and
returns `cacheSynced: false` when the bytes are unchanged
(`src/codex/catalog/remote.ts`), and `refreshCodexModelCatalog` returns
`cacheSynced: false` on both of its no-write paths with the comment "Invalidate
nothing". So the semantics were already established in this subsystem; only the
retained-sync writer had not been brought in line.
This also closes a latent defect in `pullRemoteCatalog`: it treats
`!cacheSynced` as a cache-sync failure and rolls the catalog back
(`restorePreviousCatalog` + a `write_failed` throw). Today that rollback cannot
fire, because the function returns early when the catalog bytes are unchanged, so
reaching the cache call implies the catalog changed and `cacheSynced` was
therefore reliably true. Once the cache writer reports a no-op honestly, the
rollback fires only for a genuine cache failure, which is what that branch is for.
`cacheSynced` now means what its name and its consumers already assume: a write
happened. The first pass of a change still writes, which is what keeps a guard
that simply refused every write from passing as this fix.
Refs lidge-jun#1459
…ot claim disk state changed
`invalidateCodexModelsCacheWithPermit` rewrote Codex's models cache
unconditionally, with no comparison against what was already on disk, and
returned `true` for having done so. `refreshCodexModelCatalog` reports that
boolean as `cacheSynced`, and `handleStart` ORs it into the stale-app-server
warning:
if (consumeStartupCacheInvalidationWrite() || startupSync.catalogWritten || startupSync.cacheSynced) {
warnIfStaleCodexAppServersAfterStartupWrite({ log: console });
}
So on a start where the catalog reproduced byte-identically — the settled case —
the warning asserted "Disk catalog/cache were updated" and told the operator
their Codex model list might be stale, for a write that never happened.
`writeRetainedCatalogSync` already skips a byte-identical catalog, so
`catalogWritten` is honest; `cacheSynced` was not, and it alone is enough to
raise the warning. That is the exact failure mode the catalog half of this rule
was added to stop.
This is the second writer that lidge-jun#1459 left uncovered. That issue fixed
`writeRetainedCatalogSync` with a byte-exact no-op guard, and its commit recorded
the independence deliberately — "cacheSynced is unaffected
(refreshCodexModelCatalog invalidates the models cache whenever the catalog
exists, independently of whether it was rewritten)". That is accurate: the
invalidation is attempted whenever the catalog exists, which is precisely why it
could report a write on a start that changed nothing.
The rule now lives once, in `preparedBytesDifferFromDisk`, and both writers
apply it. It stays a Buffer comparison rather than a decoded string for the
reason lidge-jun#1460 established: `readFileSync(path, "utf8")` substitutes U+FFFD for
every invalid byte, so a malformed file would decode equal to prepared content
holding a real U+FFFD, and the guard would preserve corruption while skipping
the atomic repair. An unreadable or absent file reports "differs" so the caller
performs the real write.
`pullRemoteCatalog` already guards this exact pair with a byte comparison and
returns `cacheSynced: false` when the bytes are unchanged
(`src/codex/catalog/remote.ts`), and `refreshCodexModelCatalog` returns
`cacheSynced: false` on both of its no-write paths with the comment "Invalidate
nothing". So the semantics were already established in this subsystem; only the
retained-sync writer had not been brought in line.
This also closes a latent defect in `pullRemoteCatalog`: it treats
`!cacheSynced` as a cache-sync failure and rolls the catalog back
(`restorePreviousCatalog` + a `write_failed` throw). Today that rollback cannot
fire, because the function returns early when the catalog bytes are unchanged, so
reaching the cache call implies the catalog changed and `cacheSynced` was
therefore reliably true. Once the cache writer reports a no-op honestly, the
rollback fires only for a genuine cache failure, which is what that branch is for.
`cacheSynced` now means what its name and its consumers already assume: a write
happened. The first pass of a change still writes, which is what keeps a guard
that simply refused every write from passing as this fix.
Refs lidge-jun#1459
…ot claim disk state changed
`invalidateCodexModelsCacheWithPermit` rewrote Codex's models cache
unconditionally, with no comparison against what was already on disk, and
returned `true` for having done so. `refreshCodexModelCatalog` reports that
boolean as `cacheSynced`, and `handleStart` ORs it into the stale-app-server
warning:
if (consumeStartupCacheInvalidationWrite() || startupSync.catalogWritten || startupSync.cacheSynced) {
warnIfStaleCodexAppServersAfterStartupWrite({ log: console });
}
So on a start where the catalog reproduced byte-identically — the settled case —
the warning asserted "Disk catalog/cache were updated" and told the operator
their Codex model list might be stale, for a write that never happened.
`writeRetainedCatalogSync` already skips a byte-identical catalog, so
`catalogWritten` is honest; `cacheSynced` was not, and it alone is enough to
raise the warning. That is the exact failure mode the catalog half of this rule
was added to stop.
This is the second writer that lidge-jun#1459 left uncovered. That issue fixed
`writeRetainedCatalogSync` with a byte-exact no-op guard, and its commit recorded
the independence deliberately — "cacheSynced is unaffected
(refreshCodexModelCatalog invalidates the models cache whenever the catalog
exists, independently of whether it was rewritten)". That is accurate: the
invalidation is attempted whenever the catalog exists, which is precisely why it
could report a write on a start that changed nothing.
The rule now lives once, in `preparedBytesDifferFromDisk`, and both writers
apply it. It stays a Buffer comparison rather than a decoded string for the
reason lidge-jun#1460 established: `readFileSync(path, "utf8")` substitutes U+FFFD for
every invalid byte, so a malformed file would decode equal to prepared content
holding a real U+FFFD, and the guard would preserve corruption while skipping
the atomic repair. An unreadable or absent file reports "differs" so the caller
performs the real write.
`pullRemoteCatalog` already guards this exact pair with a byte comparison and
returns `cacheSynced: false` when the bytes are unchanged
(`src/codex/catalog/remote.ts`), and `refreshCodexModelCatalog` returns
`cacheSynced: false` on both of its no-write paths with the comment "Invalidate
nothing". So the semantics were already established in this subsystem; only the
retained-sync writer had not been brought in line.
This also closes a latent defect in `pullRemoteCatalog`: it treats
`!cacheSynced` as a cache-sync failure and rolls the catalog back
(`restorePreviousCatalog` + a `write_failed` throw). Today that rollback cannot
fire, because the function returns early when the catalog bytes are unchanged, so
reaching the cache call implies the catalog changed and `cacheSynced` was
therefore reliably true. Once the cache writer reports a no-op honestly, the
rollback fires only for a genuine cache failure, which is what that branch is for.
`cacheSynced` now means what its name and its consumers already assume: a write
happened. The first pass of a change still writes, which is what keeps a guard
that simply refused every write from passing as this fix.
Refs lidge-jun#1459
…ot claim disk state changed
`invalidateCodexModelsCacheWithPermit` rewrote Codex's models cache
unconditionally, with no comparison against what was already on disk, and
returned `true` for having done so. `refreshCodexModelCatalog` reports that
boolean as `cacheSynced`, and `handleStart` ORs it into the stale-app-server
warning:
if (consumeStartupCacheInvalidationWrite() || startupSync.catalogWritten || startupSync.cacheSynced) {
warnIfStaleCodexAppServersAfterStartupWrite({ log: console });
}
So on a start where the catalog reproduced byte-identically — the settled case —
the warning asserted "Disk catalog/cache were updated" and told the operator
their Codex model list might be stale, for a write that never happened.
`writeRetainedCatalogSync` already skips a byte-identical catalog, so
`catalogWritten` is honest; `cacheSynced` was not, and it alone is enough to
raise the warning. That is the exact failure mode the catalog half of this rule
was added to stop.
This is the second writer that lidge-jun#1459 left uncovered. That issue fixed
`writeRetainedCatalogSync` with a byte-exact no-op guard, and its commit recorded
the independence deliberately — "cacheSynced is unaffected
(refreshCodexModelCatalog invalidates the models cache whenever the catalog
exists, independently of whether it was rewritten)". That is accurate: the
invalidation is attempted whenever the catalog exists, which is precisely why it
could report a write on a start that changed nothing.
The rule now lives once, in `preparedBytesDifferFromDisk`, and both writers
apply it. It stays a Buffer comparison rather than a decoded string for the
reason lidge-jun#1460 established: `readFileSync(path, "utf8")` substitutes U+FFFD for
every invalid byte, so a malformed file would decode equal to prepared content
holding a real U+FFFD, and the guard would preserve corruption while skipping
the atomic repair. An unreadable or absent file reports "differs" so the caller
performs the real write.
`pullRemoteCatalog` already guards this exact pair with a byte comparison and
returns `cacheSynced: false` when the bytes are unchanged
(`src/codex/catalog/remote.ts`), and `refreshCodexModelCatalog` returns
`cacheSynced: false` on both of its no-write paths with the comment "Invalidate
nothing". So the semantics were already established in this subsystem; only the
retained-sync writer had not been brought in line.
This also closes a latent defect in `pullRemoteCatalog`: it treats
`!cacheSynced` as a cache-sync failure and rolls the catalog back
(`restorePreviousCatalog` + a `write_failed` throw). Today that rollback cannot
fire, because the function returns early when the catalog bytes are unchanged, so
reaching the cache call implies the catalog changed and `cacheSynced` was
therefore reliably true. Once the cache writer reports a no-op honestly, the
rollback fires only for a genuine cache failure, which is what that branch is for.
`cacheSynced` now means what its name and its consumers already assume: a write
happened. The first pass of a change still writes, which is what keeps a guard
that simply refused every write from passing as this fix.
Refs lidge-jun#1459
…ot claim disk state changed (#5108) `invalidateCodexModelsCacheWithPermit` rewrote Codex's models cache unconditionally, with no comparison against what was already on disk, and returned `true` for having done so. `refreshCodexModelCatalog` reports that boolean as `cacheSynced`, and `handleStart` ORs it into the stale-app-server warning: if (consumeStartupCacheInvalidationWrite() || startupSync.catalogWritten || startupSync.cacheSynced) { warnIfStaleCodexAppServersAfterStartupWrite({ log: console }); } So on a start where the catalog reproduced byte-identically — the settled case — the warning asserted "Disk catalog/cache were updated" and told the operator their Codex model list might be stale, for a write that never happened. `writeRetainedCatalogSync` already skips a byte-identical catalog, so `catalogWritten` is honest; `cacheSynced` was not, and it alone is enough to raise the warning. That is the exact failure mode the catalog half of this rule was added to stop. This is the second writer that #1459 left uncovered. That issue fixed `writeRetainedCatalogSync` with a byte-exact no-op guard, and its commit recorded the independence deliberately — "cacheSynced is unaffected (refreshCodexModelCatalog invalidates the models cache whenever the catalog exists, independently of whether it was rewritten)". That is accurate: the invalidation is attempted whenever the catalog exists, which is precisely why it could report a write on a start that changed nothing. The rule now lives once, in `preparedBytesDifferFromDisk`, and both writers apply it. It stays a Buffer comparison rather than a decoded string for the reason #1460 established: `readFileSync(path, "utf8")` substitutes U+FFFD for every invalid byte, so a malformed file would decode equal to prepared content holding a real U+FFFD, and the guard would preserve corruption while skipping the atomic repair. An unreadable or absent file reports "differs" so the caller performs the real write. `pullRemoteCatalog` already guards this exact pair with a byte comparison and returns `cacheSynced: false` when the bytes are unchanged (`src/codex/catalog/remote.ts`), and `refreshCodexModelCatalog` returns `cacheSynced: false` on both of its no-write paths with the comment "Invalidate nothing". So the semantics were already established in this subsystem; only the retained-sync writer had not been brought in line. This also closes a latent defect in `pullRemoteCatalog`: it treats `!cacheSynced` as a cache-sync failure and rolls the catalog back (`restorePreviousCatalog` + a `write_failed` throw). Today that rollback cannot fire, because the function returns early when the catalog bytes are unchanged, so reaching the cache call implies the catalog changed and `cacheSynced` was therefore reliably true. Once the cache writer reports a no-op honestly, the rollback fires only for a genuine cache failure, which is what that branch is for. `cacheSynced` now means what its name and its consumers already assume: a write happened. The first pass of a change still writes, which is what keeps a guard that simply refused every write from passing as this fix. Refs #1459 Co-authored-by: neerajdad123-byte <neerajdad123-byte@users.noreply.github.com>
Summary
writeRetainedCatalogSyncwrote the Codex catalog unconditionally —replaceActiveCodexCatalog→atomicWriteFile, with no comparison against the bytes already on disk — so every sync moved thecatalog file's mtime even when the produced content was identical and the advertised model set did
not change.
collectCodexAppServerCatalogState()compares that mtime against each running Codex's start time.An ordinary
ocx start, or any dashboard/CLI action that re-syncs an unchanged model set, thereforeclassified every already-running Codex as holding an outdated in-memory catalog. Since #1407 that
verdict withholds all opencodex-authored v2 model guidance, so a configured
injectionModelandsubagentModelsroster silently stop reaching the session for the rest of that Codex's lifetime,even though nothing about the catalog changed. A long-lived Codex App app-server outlives every
proxy restart, which makes the state effectively latched: restarting the Codex CLI does not clear it.
This PR skips the write when the prepared bytes equal what is on disk, and reports
catalogWritten: falsefor that case.readFileSync(path, "utf8")replaces every invalid sequence with
U+FFFD, so a malformed byte on disk would compare equal toa legitimately encoded replacement character in the prepared content; the skip would then preserve
the corruption and report
catalogWritten: falsefor a file that is not what we prepared. Anunreadable or absent file still falls back to a real write.
addedstill reports the routed rows the catalog carries, because they are on disk either way.cacheSyncedis unaffected:refreshCodexModelCataloginvalidates the models cache whenever thecatalog exists, independently of whether it was rewritten. The stale-app-server hint and the
startup cache-invalidation branch both key on
cacheSyncedas well, so their behaviour isunchanged.
#1046covered warning about stale app-servers after a catalog rewrite; it did not coveravoiding the rewrite when there is nothing to write.
Closes #1459
Verification
bun x tsc --noEmit— clean.bun test tests/codex-catalog-sync-hardening.test.ts tests/codex-convergence-account-selectors.test.ts tests/codex-refresh.test.ts tests/codex-sync-api.test.ts tests/codex-catalog-restore.test.ts tests/codex-retained-root-serialization.test.ts— 72 pass, 0 fail.
bun run test(full suite) — 10871 pass, 7 skip, 1 fail. The single failure,server combo failover 030 activation matrix > connect cancellation wins with 499, no backup, warning, or cooldown(30s hook timeout), reproduces identically with this branch's changesstashed on unmodified
dev, and touches neither catalog sync nor guidance. That run predates therebase onto current
devand the byte-exactness change.bun run privacy:scan— passed.secondWritten(Expected: false, Received: true), andthe same test mutates the catalog on disk and asserts the next sync does rewrite it, so the
guard is not vacuous;
a malformed on-disk byte that decodes to the same string is still repairedonwritten(
Expected: true, Received: false). That test corrupts one byte inside a preserved JSON stringvalue to
0x80, then asserts the repaired file differs in bytes while decoding to the samestring as the corrupted file — exactly the pair a string comparison equates — and that the
result is valid UTF-8 containing
EF BF BD.@bitkyc08/opencodex2.12.0, Codex CLI 0.147.0(
originator: codex-tui), Codex App running:collectCodexAppServerCatalogState()reportedstalesolely because of the Codex App app-server (started 02:43:11Z) against a catalog whosemtime had been moved to 02:52:27Z by a resync, while both live CLI-owned processes were newer.
Checklist
section now states that a byte-identical resync leaves the file untouched.
credential, workflow, or release-automation surface is touched. The write path keeps its
existing permit check, write serialization, and atomic-write behaviour; the only change is an
equal-content short-circuit evaluated inside the same serialized region.
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
Documentation