diff --git a/AGENTS.md b/AGENTS.md index 5d094ee3..d1b785bc 100644 --- a/AGENTS.md +++ b/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 diff --git a/CHANGELOG.md b/CHANGELOG.md index 75219873..c61bece8 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/docs/CONTRACTS.md b/docs/CONTRACTS.md index da8a1022..46f66fa7 100644 --- a/docs/CONTRACTS.md +++ b/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, diff --git a/docs/architecture/models-cache-invalidation.md b/docs/architecture/models-cache-invalidation.md new file mode 100644 index 00000000..f50d14b0 --- /dev/null +++ b/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`.