Merge pull request #7626 from nesquena/release/stage-317

Release exp-v0.52.317: documentation accuracy (#7593, #7560)

AGENTS.md

@@ -5,25 +5,23 @@ repository. Keep it project-specific and safe to publish. Do not put personal
 machine setup, private network details, credentials, tokens, or local-only
 workflow notes here.
 
-## Read first
+## Documentation by task
 
-Before making changes, read:
-
-1. `README.md`
-2. `CONTRIBUTING.md`
-3. `docs/CONTRACTS.md`
-4. `CHANGELOG.md`
-
-For architecture, testing, or setup work, also read the matching reference:
+Use the references that match the change; there is no blanket reading list for
+unrelated work. Keep subsystem-specific safety and contract requirements below.
 
+- `README.md` for product behavior, setup, usage, and the docs index
+- `CONTRIBUTING.md` for contribution scope, PR format, and review requirements
+- `docs/CONTRACTS.md` to find the contract/RFC for the subsystem being changed
+- `docs/GUIDELINES.md` for bug-class coverage, state ownership, and evidence standards
+- `CHANGELOG.md` for release-history questions; ordinary PRs do not edit it
 - `ARCHITECTURE.md` for design constraints and current module layout
-- `TESTING.md` for local verification commands and manual test guidance
+- `TESTING.md` for local verification commands and relevant manual checks
 - `docs/onboarding.md` for first-run onboarding behavior
 - `docs/troubleshooting.md` for diagnostic flows
 - `docs/rfcs/README.md` for larger RFCs and state/durability contracts
-
-For UI or UX work, read `docs/UIUX-GUIDE.md` and `DESIGN.md` before
-changing layout, interaction flow, themes, chat rendering, or composer chrome.
+- `docs/UIUX-GUIDE.md` and `DESIGN.md` for layout, interaction flow, themes,
+  chat rendering, and composer chrome
 
 ## Onboarding and reinstall support
 
@@ -73,39 +71,34 @@ Follow that checklist's safety rules:
   in both the `rsync` and `cp -a` paths — `/opt/hermes` may contain subdirectories
   with restricted permissions (e.g. `.playwright/`).
 
-## Before you open a PR — the change guidelines
+## Completion and verification
 
-Read [`docs/GUIDELINES.md`](docs/GUIDELINES.md) in full before non-trivial work. It is the
-distilled set of habits that get a change merged in one review round instead of several. The
-compressed form:
+A change is ready for review when:
 
-1. **Fix the class, not the instance.** A bug usually has siblings — other call sites, backends,
-   companion endpoints, layouts, exit paths. Find them all and fix the shared chokepoint, or name
-   the ones you left out of scope.
-2. **Trace one authoritative value end-to-end** (`input → normalize → decision → action → persist →
-   cleanup`); the code that *decides* and the code that *acts* must use the same resolved value.
-3. **When you can't confirm something, fail closed and say so.** Never take the permissive branch on
-   uncertainty; never report a failure as success. "Unknown" is not "allowed."
-4. **Enumerate the state-space before editing** — entry point, backend, item count (0/1/many), every
-   lifecycle exit (success/error/cancel/replace/teardown), auth on/off, concurrency, hostile input —
-   and cover each or mark it out of scope. Most redo rounds are one un-considered dimension.
-5. **Assume inputs and check-then-use gaps are adversarial** — validate at the point of use (hold a
-   handle, don't re-resolve a path), scope caches by complete identity, handle crafted input.
-6. **A test must fail before your fix and pass after it.** Assert observable behavior, not a source
-   string or a mock of the thing under test; use multiple items if selection is what's being tested.
-7. **Name the owner of every piece of state and prove it's released on every exit** (success, error,
-   cancel, replace, shrink, teardown) — not just the happy path.
-8. **Fallbacks/defaults are contracts — extend the mechanism, don't copy it.** Editing N parallel
-   blocks identically means you missed a chokepoint (e.g. new copy goes in the `en` locale only).
-9. **The diff is the task and nothing else.** Extras go in the PR description, not the diff; run the
-   affected + neighboring tests before opening.
-10. **A visible control costs attention on every visit** — place it by frequency of use and by where
-    mainstream chat apps put the equivalent, not by where your diff already is; verify with
-    before/after images at desktop and narrow widths.
+- The requested scope is implemented, with related call sites and lifecycle
+  paths covered or explicitly identified as out of scope; unrelated cleanup is absent.
+- Behavior changes have observable regression evidence where practical, including
+  proof that a bug-fix test fails before the fix and passes after it. Evidence covers
+  affected and neighboring tests, not only new tests.
+- State changes identify the authoritative value and its owner, use that value
+  through decision, action, and persistence, and account for cleanup on every exit.
+  Fallbacks and defaults extend the existing mechanism rather than duplicate it.
+- UI changes include before/after evidence at desktop and narrow widths and
+  cover relevant mobile interactions; controls fit their frequency of use.
+- Relevant docs are current. The PR body follows `CONTRIBUTING.md`, includes
+  `Contract Routing` (and `Contract Change` for intentional contract changes)
+  where required by `docs/CONTRACTS.md`, and reports actual commands, outcomes,
+  assumptions, and anything not verified. Mocks are not proof of external behavior.
 
-Show the work in the PR body: the siblings you found, proof the test failed before the fix, the
-verification run, before/after images for visible changes, and an explicit list of what you could
-not verify.
+Within the requested scope, you may run local tests with disposable fixtures,
+fix failures caused by the change, and rerun affected tests without asking for
+approval at each step. Use `./scripts/test.sh` for pytest as specified above.
+This permission applies only with confirmed isolated state and no live
+credentials or services; the runner manages Python dependencies but is not a
+network sandbox. It does not authorize modifying real state, handling credentials,
+restarting existing services, or exposing the app beyond localhost. Those actions
+require explicit human approval and the onboarding safety rules still apply.
+If verification is blocked, report the blocker rather than claim completion.
 
 ## Local state and secrets
 
@@ -113,6 +106,11 @@ Hermes WebUI can read and write real agent state, sessions, workspaces,
 credentials, and cron data. Treat local validation as potentially destructive
 unless you have confirmed the active state directories.
 
+For authority, capability, identity, or containment checks, fail closed when
+safety cannot be confirmed: unknown is not allowed. Validate adversarial inputs
+at the point of use, account for check-then-use races, and scope caches by the
+complete identity so profiles and sessions cannot leak into each other.
+
 Prefer isolated trial state for experiments:
 
 ```bash

CHANGELOG.md

@@ -3,6 +3,11 @@
 
 ## [Unreleased]
 
+### Documentation
+
+- **`AGENTS.md` now routes contributors to the references that match their change.** The old "read first" list asked for four files up front regardless of what was being changed, and carried a compressed copy of the ten change guidelines that `docs/GUIDELINES.md` owns. It now maps each reference to the kind of work it applies to and states explicit completion/verification criteria instead. No information is lost — the ten rules remain in `docs/GUIDELINES.md`, which the new version still points to. Thanks @steveafrost. (#7593)
+- **The `/api/models` cache invalidation contract is documented.** `#7556` shipped a change to the catalog cache's source fingerprint, and its review flagged the surrounding contract as undocumented runtime behavior. `docs/architecture/models-cache-invalidation.md` now records what is cached (in-memory snapshot, per-profile `models_cache.json`, cold vs hot path), the three source axes (`config_yaml` stat identity, `auth_json` content hash with a volatile-key deny-list, baked-in plus Codex catalog hashes) and why each is fingerprinted the way it is, and the invariant that both volatile-key sets are deny-lists that may only remove fields which provably do not gate the provider/model set. Changes no runtime behavior. Thanks @webtecnica. (#7560, #7556)
+
 ### Changed
 
 - **The chat composer grows natively instead of being resized by JavaScript on every keystroke.** `autoResize()` measured `scrollHeight` and wrote `style.height` on each input event — a forced synchronous reflow on the most-typed-in surface in the app. Browsers that support CSS `field-sizing: content` (Chromium today; also Firefox 152 and Safari 26.2) now own the geometry directly, gated on `CSS.supports()`, and the existing JavaScript path is untouched for every other engine. Measured behaviour is identical across both paths: 44px resting height, no jump when the first character is typed or the last deleted, growth to the 200px ceiling, then internal scrolling. Because `field-sizing` deliberately includes placeholder text in content sizing, `:placeholder-shown` pins fixed sizing while the composer is empty so a long placeholder can't inflate it. Thanks @starship-s. (#6760, #5514)

docs/CONTRACTS.md

@@ -82,6 +82,13 @@ contributor guidance; it does not change runtime behavior or CI gates.
   proof gates. Prefer the RFC's **Authoritative emitted events** table (live
   `/api/chat/stream` wire names) over the aspirational semantic taxonomy when
   writing clients against current source.
+- [`docs/architecture/models-cache-invalidation.md`](architecture/models-cache-invalidation.md):
+  current contract for the `/api/models` catalog cache identity: the
+  `config.yaml`, `auth.json`, and catalog source-fingerprint axes, the
+  one-directional volatile-key deny-lists (auth rotation and Codex's
+  `models_cache.json` refresh timestamps), the stat-identity fallbacks, and the
+  schema/version stamps. Start here before changing model-catalog caching, the
+  `/api/models` cache keys, or the Codex catalog dependency (#2443, #7540).
 
 When a change touches streaming, recovery, replay, compression, context
 reconstruction, cancellation, approval/clarify, session metadata, or run state,

docs/architecture/models-cache-invalidation.md

@@ -0,0 +1,95 @@
+# Models catalog cache invalidation contract (`/api/models`)
+
+This document records the current identity and invalidation contract for the
+`/api/models` catalog cache in `api/config.py`. It describes shipped behavior and
+changes no runtime behavior. It was added after #7556 shipped in `exp-v0.52.303`,
+which the #7556 review flagged as an undocumented runtime contract.
+
+## What is cached
+
+- **In memory:** `_available_models_cache` plus `_available_models_cache_ts`,
+  with `_AVAILABLE_MODELS_CACHE_TTL` set to 24 hours.
+- **On disk:** one `models_cache.json` per profile under the WebUI state
+  directory (`_get_models_cache_path()`), stamped with `_schema_version` and
+  `_webui_version`.
+- **Cold path:** `get_available_models(prefer_cache=...)`; the `prefer_cache`
+  branch never starts a live provider rebuild.
+- **Hot path:** `_endpoint_advertised_model_ids()` reads only the published
+  in-memory snapshot through the lock-free `_models_cache_provenance` tuple and
+  validates it against the current source fingerprint before trusting it.
+  `_sync_models_cache_provenance()` must run at every site that publishes or
+  invalidates the snapshot, so the tuple can never tear.
+
+## The three source axes
+
+`_models_cache_source_fingerprint()` is the single chokepoint. A cache is served
+only when every axis matches the value recorded in the cache — the disk reader
+compares it in `_is_loadable_disk_cache()`, and the hot path compares it against
+the fingerprint captured at publish time.
+
+| Axis | Identity | Why it is fingerprinted this way |
+| --- | --- | --- |
+| `config_yaml` | stat identity: `mtime_ns` + size (`_models_cache_file_fingerprint`) | The file is rewritten only on deliberate user edits, and any edit can change the provider/model set, so the cheap conservative identity wins. |
+| `auth_json` | content hash with a volatile-key deny-list (`_auth_store_semantic_fingerprint`, `_AUTH_FINGERPRINT_VOLATILE_KEYS`) | The credential store is rewritten roughly every 14 minutes by credential-pool / OAuth refresh; none of those rotating fields feed `detected_providers` or the returned catalog, and stat identity made the 24h cache churn on every refresh (RCA `t_d127953d` / `t_16551f61`). |
+| `catalog` | baked-in provider catalog sha256 (`_PROVIDER_MODELS` + `_PROVIDER_DISPLAY`) plus the Codex local catalog (`_codex_models_cache_fingerprint`, `_CODEX_CACHE_FINGERPRINT_VOLATILE_KEYS`) | A restart after a catalog change must not keep serving a persisted payload for up to 24h (#2443). Codex rewrites `~/.codex/models_cache.json` on its own timer, bumping `mtime_ns` and size while models, `etag`, and `client_version` stay identical, so the Codex axis hashes **content** with only the refresh timestamps (`fetched_at`, `updated_at`) removed (#7540, #7556). |
+
+## Invariant: deny-lists are one-directional
+
+- Both volatile-key sets are deny-lists, never allow-lists. They may remove only
+  fields that provably do not gate the provider/model set.
+- Every other field — including fields that Codex or the auth store may add in
+  the future — stays in the fingerprint
+  (`test_unknown_codex_field_stays_in_fingerprint`).
+- Consequence: excluding a volatile key can only make the fingerprint **more
+  stable**; it can never hide a genuine catalog or provider change. When in
+  doubt, keep the key in.
+
+## Invariant: fallbacks are never less safe than stat
+
+- Missing file → recorded as missing, and the fingerprint stays stable
+  (`test_missing_codex_cache_fingerprint_is_stable_and_marked`).
+- Unreadable, corrupt, or mid-write JSON → stat identity, marked
+  `unparsed-fallback`.
+- Transform failure — including `RecursionError` on a pathologically deep tree →
+  stat identity, marked `encode-fallback`
+  (`test_deeply_nested_codex_cache_degrades_to_stat_fallback_without_crashing`).
+  The fingerprint must never raise into `/api/models`; a real rewrite still
+  changes the stat identity, so the fallback is strictly no less safe than the
+  pre-#7556 behavior.
+
+## Version stamps (independent of the fingerprint)
+
+- `_schema_version` must equal `_MODELS_CACHE_SCHEMA_VERSION` (currently `3`).
+  Bump it when the cached payload shape changes incompatibly.
+- `_webui_version` must equal the running version, which forces a rebuild after
+  every release so picker-shape fixes appear immediately instead of after the
+  TTL expires. When the runtime version cannot be resolved (early boot), that
+  check is skipped rather than wedging the boot.
+
+## Change protocol
+
+1. Add or change a source axis in `_models_cache_source_fingerprint()` only —
+   one chokepoint, so the disk reader and the published-snapshot reader always
+   agree.
+2. When adding a volatile key, name the value it derives from and prove it
+   inert: one test that fails if the key is dropped from the deny-list, and one
+   that shows a genuine catalog change still invalidates the cache.
+3. Keep the fingerprint cheap, deterministic, and safe to run synchronously —
+   it is recomputed on cache reads, including a per-turn hot path.
+4. Do not add a second, parallel cache-identity mechanism. The stat-based
+   `_models_cache_file_fingerprint()` stays only as the conservative fallback for
+   the axes above.
+5. This is a runtime contract: changes here update this document and are
+   described in the PR body. Release-note wording belongs in the PR body, not in
+   `CHANGELOG.md`, which release commits own.
+
+## Tests
+
+`tests/test_issue7540_codex_catalog_fingerprint.py` covers both invariant groups:
+timestamp-only churn keeps the fingerprint identical (and a session visit after a
+Codex refresh needs no live rebuild), while genuine changes — a new model, a
+visibility change, any catalog field, any unknown field — still invalidate.
+
+## References
+
+Issues/PRs: #2443, #7540, #7556, #7558. RCAs: `t_d127953d`, `t_16551f61`.