diff --git a/Cargo.lock b/Cargo.lock index 851f976ce..f58141cca 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -57,8 +57,8 @@ checksum = "683d7910e743518b0e34f1186f92494becacb047c7b6bf616c96772180fef923" [[package]] name = "amplihack-agent-eval" -version = "0.11.1" -source = "git+https://github.com/rysweet/amplihack-rs.git?rev=14dc30b10e87764120c6f2bae7f3630522c29e5d#14dc30b10e87764120c6f2bae7f3630522c29e5d" +version = "0.18.0" +source = "git+https://github.com/rysweet/amplihack-rs.git?rev=9ee05a06eab98e9ab504a031bffaa4190700c2af#9ee05a06eab98e9ab504a031bffaa4190700c2af" dependencies = [ "chrono", "serde", diff --git a/Cargo.toml b/Cargo.toml index 8b4461271..e4b051887 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -154,11 +154,53 @@ rustyclawd-tools = { git = "https://github.com/rysweet/RustyClawd.git", rev = "d # wiring (`src/gym_runner_client.rs`) and Simard-specific scoring/history stay # in this crate. The crate is a light workspace member (serde/serde_json/ # thiserror/tracing/chrono only — no heavy workspace deps are transitive). -# Bumped (issue #2626) to the current HEAD of amplihack-rs `main` (verified via -# `git ls-remote`); the delta is workflow/recipe/CI-tooling fixes only, with the -# `amplihack-agent-eval` crate surface (GymRunner/GymConfig/GymScenarioResult) -# and the reverted workspace version 0.11.1 unchanged. -amplihack-agent-eval = { git = "https://github.com/rysweet/amplihack-rs.git", rev = "14dc30b10e87764120c6f2bae7f3630522c29e5d" } +# Pin lineage (each rev is an immutable amplihack-rs commit SHA): +# 59548a96… (initial de-fork, #2323) +# -> 2a93441d… landed by issue #2626 — bump both amplihack pins to the +# then-current upstream `main` HEAD; +# -> 14dc30b1… landed by issue #2767 — pick up amplihack-rs PR #856, the +# clean agent-result channel off the noisy stdout (a 2026-07-07 UTC +# commit: 2026-07-07 01:47:01 UTC / 2026-07-06 18:47:01 -0700); +# -> 9ee05a06… (this pin, below). +# Across 2a93441d…/14dc30b1… the delta was workflow/recipe/CI-tooling only, with +# the `amplihack-agent-eval` crate surface (GymRunner/GymConfig/ +# GymScenarioResult) unchanged. +# +# This pin is the amplihack-rs **v0.18.25** release source commit +# `9ee05a06eab98e9ab504a031bffaa4190700c2af` (release published 2026-09-02; +# pin verified 2026-09-03). Provenance, all verified with `git ls-remote` / +# `git cat-file` against https://github.com/rysweet/amplihack-rs.git: +# * the **annotated** tag `refs/tags/v0.18.25` is object `e947170a…`, and it +# dereferences (`refs/tags/v0.18.25^{}`) to commit `9ee05a06…`; +# * `refs/heads/main` *was* `9ee05a06…` **at verification on 2026-09-03** — +# i.e. the release tag and `main` pointed at the same commit at that +# instant, which establishes that `9ee05a06…` is a mainline release commit +# rather than a side/topic branch. This is a point-in-time observation, not +# a standing claim: upstream `main` advances, and once it does this comment +# stays true (it records what was observed then) while the pin deliberately +# does not move. +# * `9ee05a06…` was 200 commits ahead of the previous pin `14dc30b1…` at that +# same verification. +# The pin stays the immutable commit SHA (never the tag) so the build cannot be +# re-pointed by an upstream tag move. +# +# API impact: NONE. The `crates/amplihack-agent-eval` subtree is **byte +# identical** across those 200 commits — both `14dc30b1…:crates/amplihack-agent-eval` +# and `9ee05a06…:crates/amplihack-agent-eval` resolve to git tree +# `1a635333f15fa2964e6d0ecae35b5a9625d59ee7`, and the crate's own dependency set +# (serde/serde_json/thiserror/tracing/chrono) is unchanged. So +# `src/gym_runner_client.rs` needs no adaptation and the wire JSON stays +# byte-stable. +# +# Lockfile impact: the crate inherits `version.workspace`, which upstream moved +# 0.11.1 -> 0.18.0 in the workspace manifest, so Cargo.lock records +# `amplihack-agent-eval 0.18.0` at the new rev. NOTE the deliberate mismatch: +# the upstream *release* is tagged v0.18.25 while the upstream *workspace crate +# version* at `9ee05a06…` is 0.18.0 — the tag tracks the release train, not the +# crate version. Both readings are pinned to that immutable commit, so neither +# changes as upstream advances. Simard consumes the crate by rev, so only the +# rev is authoritative. +amplihack-agent-eval = { git = "https://github.com/rysweet/amplihack-rs.git", rev = "9ee05a06eab98e9ab504a031bffaa4190700c2af" } # De-fork phase 2b (issue #2307): the native cognitive-memory fork is gone, but # `lbug` is still a direct dependency of the standalone `simard-tui` binary # (`src/bin/simard_tui/goals.rs`), which opens a LadybugDB read-only to render diff --git a/docs/architecture/gym-eval-library-adapter.md b/docs/architecture/gym-eval-library-adapter.md index c9bd19508..f94ea9158 100644 --- a/docs/architecture/gym-eval-library-adapter.md +++ b/docs/architecture/gym-eval-library-adapter.md @@ -1,7 +1,7 @@ --- title: Library-backed Gym Evaluation Engine (the sole engine) description: How Simard's gym client is backed by the amplihack-agent-eval crate's native Rust GymRunner through the thin gym_runner_client adapter. As of the de-fork the private native_gym reimplementation has been deleted and the library is the only evaluation engine. -last_updated: 2026-06-20 +last_updated: 2026-09-03 owner: simard doc_type: reference related: @@ -64,11 +64,15 @@ three handlers moved into the library. ## The dependency The engine is a pinned-revision git dependency in `Cargo.toml`, matching the -immutable-rev style already used for `amplihack-memory` and `rustyclawd-core`: +immutable-rev style already used for `amplihack-memory` and `rustyclawd-core`. +The authoritative pin is always the live line in the root `Cargo.toml`; the +snippet below records the rev current as of **2026-09-03** — the amplihack-rs +`v0.18.25` release source commit (the annotated tag `v0.18.25` dereferences to +it, and it was `main` HEAD at verification): ```toml [dependencies] -amplihack-agent-eval = { git = "https://github.com/rysweet/amplihack-rs.git", rev = "59548a96049ab8d558110bcaf9c82a4316f1bbf0" } +amplihack-agent-eval = { git = "https://github.com/rysweet/amplihack-rs.git", rev = "9ee05a06eab98e9ab504a031bffaa4190700c2af" } ``` ### Consumability @@ -195,11 +199,19 @@ scenario list. The wire `scenario_id` is therefore always one of the ids `gym.list_scenarios` advertises — never the bare `"L{n}"` form. **2. Suite `success`.** The library's `run_suite` computes its top-level -`success` with inverted logic — +`success` as `!result.failed_levels.is_empty() || result.level_results.iter().all(|lr| lr.success)` -(`amplihack_agent_eval::gym::GymRunner::run_suite`, gym.rs:300-301) — which -evaluates to `true` precisely when there *are* failed levels. The adapter does -**not** trust that flag; it recomputes +(`amplihack_agent_eval::gym::GymRunner::run_suite`). This is a **tautology: it +always evaluates to `true`.** `ProgressiveResult::add_result` records a level id +in `failed_levels` exactly when that level's result is *not* successful, so an +empty `failed_levels` implies every entry of `level_results` succeeded — meaning +whenever the first disjunct is `false`, the second is `true`. The flag is +therefore `true` on an all-pass suite, on a partially-failing suite, and on an +empty suite (`all()` over an empty iterator is `true`). + +The consequence is stronger than "the flag is inverted": it carries **no +information at all**, so there is nothing to invert or correct. The adapter +ignores it outright and recomputes `success = scenarios_passed == scenarios_total` from the per-scenario results it already maps. (This upstream quirk should be filed as an issue against `amplihack-rs`; until it is fixed, the adapter's recomputation is the contract diff --git a/docs/atlas/compile-deps/README.md b/docs/atlas/compile-deps/README.md index 78ef76cfa..ea15afaa0 100644 --- a/docs/atlas/compile-deps/README.md +++ b/docs/atlas/compile-deps/README.md @@ -1,3 +1,13 @@ +--- +title: Compile Dependencies Atlas +description: "Code-derived atlas layer mapping Simard's direct Cargo dependencies (including the exact git-rev pins) and a one-hop view of the largest internal Rust module dependencies." +last_updated: 2026-09-03 +review_schedule: as-needed +owner: simard +doc_type: reference +status: living document — regenerate from code when stale +--- + # Compile Dependencies Atlas This layer maps Simard's direct Cargo dependencies and a one-hop view of the largest internal Rust module dependencies. The overview diagrams stay small by grouping the external crates, dev/build dependencies, and internal module graph; the split diagrams below expand those groups without exceeding the atlas density target. @@ -22,20 +32,40 @@ This layer maps Simard's direct Cargo dependencies and a one-hop view of the lar ## Evidence anchors -- Direct dependencies are declared in `Cargo.toml:135` through `Cargo.toml:209`; Unix target dependency is at `Cargo.toml:212`; build and dev dependencies are at `Cargo.toml:218` and `Cargo.toml:245` through `Cargo.toml:254`. -- `amplihack-memory` is pinned to `amplihack-memory-lib` with the `persistent` feature at `Cargo.toml:135`; the direct `lbug` fork pin is at `Cargo.toml:158`. -- The default feature set enables `signal` and `dashboard-audit` at `Cargo.toml:227`, and `dashboard-audit` pulls `headless_chrome`, `regex`, and `url` at `Cargo.toml:236`. -- The large internal module declarations are visible in `src/lib.rs:24`, `src/lib.rs:60`, `src/lib.rs:78`, `src/lib.rs:89`, `src/lib.rs:90`, `src/lib.rs:103`, `src/lib.rs:106`, `src/lib.rs:114`, `src/lib.rs:117`, `src/lib.rs:122`, `src/lib.rs:123`, `src/lib.rs:124`, `src/lib.rs:126`, `src/lib.rs:128`, `src/lib.rs:138`, `src/lib.rs:163`, and `src/lib.rs:164`. +Anchors are **section- and symbol-based**, not line numbers: `Cargo.toml` and +`src/lib.rs` both carry long provenance comment blocks, so raw line numbers rot +on every pin bump (they were already stale before this page was last touched). +Each anchor below is a stable, greppable construct. + +- Direct dependencies are declared under the `[dependencies]` table in + `Cargo.toml`; the Unix-only dependency (`libc`) is under + `[target.'cfg(unix)'.dependencies]`; build and dev dependencies are under + `[build-dependencies]` (`chrono`) and `[dev-dependencies]` (`assert_cmd`, + `proptest`, `serial_test`, `syn`, `proc-macro2`). +- `amplihack-memory` is pinned to `amplihack-memory-lib` with the `persistent` + feature on the `amplihack-memory = { git = …, features = ["persistent"] }` + key; the direct `lbug` fork pin is the `lbug = { git = …, rev = … }` key. Both + are in `[dependencies]` — grep the key names. +- The `[features]` table sets `default = ["signal", "dashboard-audit"]`, and + `dashboard-audit = ["dep:headless_chrome", "dep:regex", "dep:url"]`. +- The large internal modules the module-graph diagram draws are the `pub mod` / + `mod` declarations in `src/lib.rs` — e.g. `cognitive_memory`, `goal_curation`, + `ooda_loop`, `ooda_actions`, `ooda_brain`, `journal`, `overseer`, + `meeting_backend`, `memory_consolidation`, `memory_ipc`, `engineer_loop`, + `self_deploy`, `self_improve`, and `runtime`. Every node name in + `compile-deps-internal-modules.dot` matches a `mod ;` declaration in + `src/lib.rs`, so the diagram can be re-checked with + `grep -E '^(pub )?mod ;' src/lib.rs`. ## Dependency inventory | Crate or section | Version or pin | Purpose | | --- | --- | --- | -| `amplihack-memory` | git `c266e15d1399967c04324370e77cf281990b8be1`, feature `persistent` | Sole cognitive-memory backend adapter; persistent graph storage via upstream memory library. | +| `amplihack-memory` | git `0031505b911151bf47409694a6c45f8b778d91b9`, feature `persistent` | Sole cognitive-memory backend adapter; persistent graph storage via upstream memory library. | | `lbug` | git `5a2c107881879f4d1bb594b14967948870e65cdc` | Embedded LadybugDB graph store used directly by TUI and unified with `amplihack-memory`. | | `rustyclawd-core` | git `dcccad80ed381c66a7728565be5cb84120aacbed` | RustyClawd core agent SDK integration. | | `rustyclawd-tools` | git `dcccad80ed381c66a7728565be5cb84120aacbed` | RustyClawd tool integration. | -| `amplihack-agent-eval` | git `14dc30b10e87764120c6f2bae7f3630522c29e5d` | Native Rust gym and evaluation runner types. | +| `amplihack-agent-eval` | git `9ee05a06eab98e9ab504a031bffaa4190700c2af` (amplihack-rs release `v0.18.25`) | Native Rust gym and evaluation runner types. | | `rusqlite` | `=0.31.0`, features `backup`, `bundled` | SQLite storage and backup support. | | `serde` | `=1.0.228`, feature `derive` | Serialization derives for config, state, and message types. | | `serde_json` | `=1.0.149` | JSON IO for CLI, recipes, telemetry payloads, and stored records. | diff --git a/docs/atlas/compile-deps/compile-deps-external-runtime-dot.svg b/docs/atlas/compile-deps/compile-deps-external-runtime-dot.svg index 21dc3a116..a4f7845b5 100644 --- a/docs/atlas/compile-deps/compile-deps-external-runtime-dot.svg +++ b/docs/atlas/compile-deps/compile-deps-external-runtime-dot.svg @@ -21,7 +21,7 @@ amplihack_memory amplihack-memory -git c266e15 persistent +git 0031505 persistent @@ -60,7 +60,7 @@ amplihack_agent_eval amplihack-agent-eval -git 14dc30b +git 9ee05a0 diff --git a/docs/atlas/compile-deps/compile-deps-external-runtime-mermaid.svg b/docs/atlas/compile-deps/compile-deps-external-runtime-mermaid.svg index 34fb2f852..813414626 100644 --- a/docs/atlas/compile-deps/compile-deps-external-runtime-mermaid.svg +++ b/docs/atlas/compile-deps/compile-deps-external-runtime-mermaid.svg @@ -1 +1 @@ -

simard

amplihack-memory git c266e15 persistent

rustyclawd-core git dcccad8

rustyclawd-tools git dcccad8

amplihack-agent-eval git 14dc30b

lbug git 5a2c107

rusqlite =0.31.0 backup bundled

serde =1.0.228 derive

serde_json =1.0.149

semver =1.0.28

ureq =3.3.0

toml =1.1.2

tokio =1.52.1 rt process net macros sync

axum =0.8.9 ws

tower-http =0.6.8 cors auth

uuid =1.23.4 v4 v7

chrono =0.4.45

chrono-tz =0.10.4

dirs =6.0.0

crc32fast =1.5.0

sha2 =0.10.9

tracing =0.1.44

tracing-subscriber =0.3.23 json env-filter

ctrlc =3.5.2 termination

nix =0.29.0 signal fs

opentelemetry =0.27.1

opentelemetry-otlp =0.27.0

tracing-opentelemetry =0.28.0

opentelemetry_sdk =0.27.1 rt-tokio

tempfile =3.27.0

ratatui =0.29.0

crossterm =0.28.1

headless_chrome =1.0.21 optional

regex =1.12.3 optional

url =2.5.8 optional

base64 =0.22.1

rustls =0.23.38 ring

rustls-native-certs =0.8.4

webpki-roots =1.0.7

libc =0.2.185 unix target

 \ No newline at end of file +

simard

amplihack-memory git 0031505 persistent

rustyclawd-core git dcccad8

rustyclawd-tools git dcccad8

amplihack-agent-eval git 9ee05a0

lbug git 5a2c107

rusqlite =0.31.0 backup bundled

serde =1.0.228 derive

serde_json =1.0.149

semver =1.0.28

ureq =3.3.0

toml =1.1.2

tokio =1.52.1 rt process net macros sync

axum =0.8.9 ws

tower-http =0.6.8 cors auth

uuid =1.23.4 v4 v7

chrono =0.4.45

chrono-tz =0.10.4

dirs =6.0.0

crc32fast =1.5.0

sha2 =0.10.9

tracing =0.1.44

tracing-subscriber =0.3.23 json env-filter

ctrlc =3.5.2 termination

nix =0.29.0 signal fs

opentelemetry =0.27.1

opentelemetry-otlp =0.27.0

tracing-opentelemetry =0.28.0

opentelemetry_sdk =0.27.1 rt-tokio

tempfile =3.27.0

ratatui =0.29.0

crossterm =0.28.1

headless_chrome =1.0.21 optional

regex =1.12.3 optional

url =2.5.8 optional

base64 =0.22.1

rustls =0.23.38 ring

rustls-native-certs =0.8.4

webpki-roots =1.0.7

libc =0.2.185 unix target

 \ No newline at end of file diff --git a/docs/atlas/compile-deps/compile-deps-external-runtime.dot b/docs/atlas/compile-deps/compile-deps-external-runtime.dot index ca0a83f31..f03740320 100644 --- a/docs/atlas/compile-deps/compile-deps-external-runtime.dot +++ b/docs/atlas/compile-deps/compile-deps-external-runtime.dot @@ -5,10 +5,10 @@ digraph compile_deps_external_runtime { simard [label="simard"]; - amplihack_memory [label="amplihack-memory\ngit c266e15 persistent"]; + amplihack_memory [label="amplihack-memory\ngit 0031505 persistent"]; rustyclawd_core [label="rustyclawd-core\ngit dcccad8"]; rustyclawd_tools [label="rustyclawd-tools\ngit dcccad8"]; - amplihack_agent_eval [label="amplihack-agent-eval\ngit 14dc30b"]; + amplihack_agent_eval [label="amplihack-agent-eval\ngit 9ee05a0"]; lbug [label="lbug\ngit 5a2c107"]; rusqlite [label="rusqlite =0.31.0\nbackup bundled"]; diff --git a/docs/atlas/compile-deps/compile-deps-external-runtime.mmd b/docs/atlas/compile-deps/compile-deps-external-runtime.mmd index 785e3eee2..e10933ed8 100644 --- a/docs/atlas/compile-deps/compile-deps-external-runtime.mmd +++ b/docs/atlas/compile-deps/compile-deps-external-runtime.mmd @@ -1,8 +1,8 @@ flowchart LR - simard["simard"] --> amplihack_memory["amplihack-memory git c266e15 persistent"] + simard["simard"] --> amplihack_memory["amplihack-memory git 0031505 persistent"] simard --> rustyclawd_core["rustyclawd-core git dcccad8"] simard --> rustyclawd_tools["rustyclawd-tools git dcccad8"] - simard --> amplihack_agent_eval["amplihack-agent-eval git 14dc30b"] + simard --> amplihack_agent_eval["amplihack-agent-eval git 9ee05a0"] simard --> lbug["lbug git 5a2c107"] simard --> rusqlite["rusqlite =0.31.0 backup bundled"] simard --> serde["serde =1.0.228 derive"] diff --git a/docs/howto/self-maintain-dependency-pins.md b/docs/howto/self-maintain-dependency-pins.md index 652d019c7..98f8ffa62 100644 --- a/docs/howto/self-maintain-dependency-pins.md +++ b/docs/howto/self-maintain-dependency-pins.md @@ -1,7 +1,7 @@ --- title: How to keep Simard's own dependency pins up to date description: "A reactive done-gate and proactive reconcile that make Simard bump her own Cargo.toml git-rev pins after she lands a change upstream, so the fixes she ships actually run in her own daemon. Prompt-first, no Rust logic." -last_updated: 2026-06-26 +last_updated: 2026-09-03 review_schedule: as-needed owner: simard doc_type: howto @@ -62,16 +62,17 @@ CLI command, and no change to any parser contract. ```mermaid flowchart TD subgraph A[Reactive done-gate] - A1([engineer lands upstream change]) --> A2[bump own Cargo.toml rev
to merged main commit] + A1([engineer lands upstream change]) --> A2["bump own Cargo.toml rev
to that exact merge commit SHA"] A2 --> A3[cargo build verifies] A3 --> A4[open / update bump PR
vs rysweet/Simard] A4 --> A5[land bump PR] A5 --> A6([goal DONE]) end subgraph B[Proactive reconcile] - B1([idle / research time]) --> B2[compare each pinned rev
to upstream default HEAD] - B2 -->|behind| B3[open / update bump follow-up] - B2 -->|current| B4([nothing to do]) + B1([idle / research time]) --> B2["compare each pinned rev
to upstream default HEAD
base=pin head=main → ahead_by"] + B2 -->|"ahead_by > 0 AND a newer
approved target exists"| B3[open / update bump follow-up
targeting that exact commit] + B2 -->|"ahead_by > 0 but no newer
approved target"| B5([drift is informational
nothing to do]) + B2 -->|"identical"| B4([nothing to do]) end ``` @@ -92,7 +93,10 @@ one of the three repos Simard pins by git rev — `amplihack-rs`, Simard has also: 1. **Bumped her own pin.** Edit the matching `rev = "…"` in the root - `Cargo.toml` to the merged `main` commit SHA. + `Cargo.toml` to the **exact immutable SHA of the merge commit that carried + that change** — that specific commit is the approved target here. It is *not* + "whatever `main` points at now": `main` may already have moved on, and the + pin must not chase it. 2. **Verified the build.** `cargo build` (low-space variant when disk is tight — `scripts/cargo-low-space build`) must succeed against the new rev. A bump that does not build is rolled back, not shipped. @@ -149,27 +153,84 @@ low-priority follow-up precisely because nothing is holding a goal open. ## Trigger B — the proactive reconcile When Simard has low-priority idle/research time (the same budget she uses for -self-maintenance), she periodically checks each rev-pinned git dependency for -**drift**: is the pinned rev behind the upstream default branch? - -For each dependency she compares the pinned rev to the upstream `HEAD`: +self-maintenance), she periodically measures each rev-pinned git dependency's +**drift**: how many commits has the upstream default branch advanced *past* the +pinned rev? + +For each dependency she compares the pinned rev to the upstream `HEAD`. + +> **Read the compare orientation carefully.** The GitHub compare API is +> `compare/{base}...{head}`, and `ahead_by` / `behind_by` are reported **from +> the point of view of `head`**. With `base = $PINNED` and `head = main`: +> +> * `ahead_by` = how many commits **`main` is ahead of the pin** — this is the +> drift number you want; +> * `behind_by` = how many commits `main` is *behind* the pin. Whenever the pin +> is an ancestor of `main` (the normal case) this is **always `0`**; +> * `status` is therefore `"ahead"` for a pin that trails `main`, never +> `"behind"`, and `"identical"` only when the pin *is* `main`'s HEAD. +> +> Reading `.behind_by` in this orientation is a silent no-op: it prints `0` +> forever and the drift check can never fire. Use `.ahead_by`. (To get a +> `behind_by`-shaped answer you would have to flip the operands to +> `compare/main...$PINNED`.) ```bash -# Current pin (from Cargo.toml) -PINNED=59548a96049ab8d558110bcaf9c82a4316f1bbf0 - -# Upstream default-branch HEAD -git ls-remote https://github.com/rysweet/amplihack-rs.git main - -# How far behind is the pin? (GitHub compare API: base=pin, head=main) -gh api repos/rysweet/amplihack-rs/compare/$PINNED...main \ - --jq '{status, behind: .behind_by, ahead: .ahead_by}' +# Current pin — read it from the ONE live dependency line rather than +# hardcoding a rev here, which would go stale at the next bump. +# * `^` anchors to the start of the line, so the `#` provenance comments +# (which also carry 40-char SHAs, including git *tree* hashes) can never +# match; +# * the `rev = "…"` capture takes exactly one 40-char hex value; +# * the count check refuses to continue on zero or multiple matches instead +# of silently pasting two concatenated SHAs into the compare URL. +PINNED=$(sed -n 's/^amplihack-agent-eval[[:space:]]*=.*,[[:space:]]*rev[[:space:]]*=[[:space:]]*"\([0-9a-f]\{40\}\)".*/\1/p' Cargo.toml) + +if [ "$(printf '%s\n' "$PINNED" | grep -c '^[0-9a-f]\{40\}$')" -ne 1 ]; then + echo "ERROR: expected exactly one amplihack-agent-eval rev, got: '$PINNED'" >&2 +else + # Upstream default-branch HEAD + git ls-remote https://github.com/rysweet/amplihack-rs.git main + + # How far has `main` advanced past the pin? base=pin, head=main, so the + # answer is `ahead_by` (main ahead of pin); `behind_by` is always 0 here. + # "$PINNED" is quoted so a malformed value can never word-split the path. + gh api "repos/rysweet/amplihack-rs/compare/$PINNED...main" \ + --jq '{status, main_ahead_of_pin: .ahead_by}' +fi ``` -`status: "behind"` (or `ahead > 0`) means the pin is stale. Simard then opens — -or updates — a **bump follow-up** that does exactly what Trigger A does: -re-point the rev to the new `main`, verify `cargo build`, and ship it through -the normal landing pipeline. +`status: "ahead"` with `main_ahead_of_pin > 0` means `main` has moved on since +the pin. `status: "identical"` with `main_ahead_of_pin == 0` means the pin *is* +the current `main` HEAD. + +### Drift is a signal, not an automatic bump + +**A non-zero drift count does not by itself mean the pin is wrong.** Simard +pins **immutable commit SHAs**, and for `amplihack-agent-eval` the pin targets a +**tagged release commit** (currently the `v0.18.25` source commit — see the +provenance block on the `amplihack-agent-eval` line in the root `Cargo.toml`). +Upstream `main` advances continuously, so a release pin starts drifting the +moment the next commit lands upstream — that is normal and expected, not a +defect. + +So the reconcile asks two questions, in order: + +1. **Is there a newer *approved target*?** For a release-pinned crate that means + a newer **release tag** that has been reviewed and chosen for adoption; for a + crate deliberately tracking `main`, it means the specific merged commit that + carries the wanted fix. If there is no such target, drift is **informational + only** and there is nothing to do. +2. **If yes, adopt that exact target** — never "whatever `main` happens to be + right now". Simard then opens (or updates) a **bump follow-up** that does + what Trigger A does: re-point the rev to the chosen commit, verify + `cargo build`, and ship it through the normal landing pipeline. + +Concretely: the drift number tells Simard *how much* upstream has moved and is +useful for deciding whether a review is worth scheduling. It is **not** a +tripwire that must be driven back to zero, and a pin is **not stale merely +because `ahead_by > 0`**. Chasing every `main` commit would defeat the point of +pinning — reproducible builds on a reviewed, release-quality commit. This trigger is **low priority by construction**. It never preempts an active engineering goal; it only fills spare capacity. It is described in @@ -225,9 +286,11 @@ gh pr list --repo rysweet/Simard --state open \ --json number,title,headRefName ``` -- **Found** → **update it**: re-point every crate from that repo to the latest - `main`, re-run `cargo build`, force-update the branch, and refresh the PR - body. Do **not** open a second PR. +- **Found** → **update it**: re-point every crate from that repo to the **newly + selected approved target commit** — the reviewed release commit, or the + specific merged commit carrying the required fix — **never "the latest + `main`"**. Then re-run `cargo build`, force-update the branch, and refresh the + PR body. Do **not** open a second PR. - **Not found** → open a fresh PR with the convention above. > **Cross-repo safety.** The upstream change lives in another repo @@ -298,21 +361,35 @@ as above. grep -nE 'git = .*(amplihack-rs|amplihack-memory-lib|RustyClawd)' Cargo.toml ``` -2. **Check one dependency for drift:** +2. **Measure one dependency's drift from `main`:** ```bash - PINNED=$(grep 'amplihack-agent-eval' Cargo.toml | grep -oE '[0-9a-f]{40}') - gh api repos/rysweet/amplihack-rs/compare/$PINNED...main --jq '.behind_by' + # Anchored at `^` so the `#` provenance comments (which also contain + # 40-char SHAs) cannot match; the count check refuses zero-or-many. + PINNED=$(sed -n 's/^amplihack-agent-eval[[:space:]]*=.*,[[:space:]]*rev[[:space:]]*=[[:space:]]*"\([0-9a-f]\{40\}\)".*/\1/p' Cargo.toml) + + if [ "$(printf '%s\n' "$PINNED" | grep -c '^[0-9a-f]\{40\}$')" -ne 1 ]; then + echo "ERROR: expected exactly one amplihack-agent-eval rev, got: '$PINNED'" >&2 + else + # base=pin, head=main => `.ahead_by` is how far main is AHEAD of the pin. + # `.behind_by` is always 0 in this orientation — reading it is a no-op. + gh api "repos/rysweet/amplihack-rs/compare/$PINNED...main" --jq '.ahead_by' + fi ``` - A non-zero result means the pin is stale and Trigger B should produce a bump - follow-up. + A non-zero result means `main` has advanced past the pin. That is + **informational**: see + [Drift is a signal, not an automatic bump](#drift-is-a-signal-not-an-automatic-bump). + It becomes a Trigger-B bump follow-up only when a newer **approved target** + (for `amplihack-agent-eval`, a newer reviewed **release tag**) should be + adopted — not merely because the counter is non-zero. -3. **After a bump lands, confirm the rev moved and the build is green:** +3. **After a bump lands, confirm the rev moved to the chosen target and the + build is green:** ```bash git -C ~/src/Simard pull - grep -n 'amplihack-agent-eval' ~/src/Simard/Cargo.toml # rev now == merged main + grep -n 'amplihack-agent-eval' ~/src/Simard/Cargo.toml # rev == the adopted target commit cargo build --quiet # succeeds ``` diff --git a/docs/reference/amplihack-pin-bump-2626.md b/docs/reference/amplihack-pin-bump-2626.md index 1dfb3511c..3042cb6d1 100644 --- a/docs/reference/amplihack-pin-bump-2626.md +++ b/docs/reference/amplihack-pin-bump-2626.md @@ -1,11 +1,11 @@ --- title: amplihack pin bump to upstream main (#2626) -description: "Reference record for the issue #2626 dependency-pin reconcile that bumped Simard's amplihack-agent-eval and amplihack-memory git-rev pins from behind-main revisions to the current upstream main HEADs, with the lbug lockstep, API-parity, and supply-chain re-verification that gated the bump." -last_updated: 2026-07-06 +description: "Historical record of the issue #2626 dependency-pin reconcile that bumped Simard's amplihack-agent-eval and amplihack-memory git-rev pins from behind-main revisions to the then-current upstream main HEADs, with the lbug lockstep, API-parity, and supply-chain re-verification that gated the bump. The verification method remains current; the rev table and the track-current-main target policy are superseded." +last_updated: 2026-09-03 review_schedule: as-needed owner: simard doc_type: reference -status: active +status: historical — superseded; the live pin is the `amplihack-agent-eval` line in the root `Cargo.toml` related: - ../howto/self-maintain-dependency-pins.md - ../architecture/gym-eval-library-adapter.md @@ -16,14 +16,49 @@ related: # amplihack pin bump to upstream main (#2626) -> **Status: active.** This page is the completed change record for issue +> **Status: historical — superseded.** This page is the completed change record +> for issue > [#2626](https://github.com/rysweet/Simard/issues/2626): the reconcile that > re-pointed Simard's two `amplihack-*` git-rev pins from stale, behind-`main` -> revisions to the current upstream `main` HEADs, so the fixes those upstream -> repos already merged actually run in Simard's own build. It is a concrete, -> worked instance of the **proactive reconcile (Trigger B)** described in +> revisions to the then-current upstream `main` HEADs, so the fixes those +> upstream repos already merged actually run in Simard's own build. It is a +> concrete, worked instance of the **proactive reconcile (Trigger B)** described in > [How to keep Simard's dependency pins up to date](../howto/self-maintain-dependency-pins.md), -> and it doubles as the specification the bump PR is verified against. +> and it doubled as the specification the #2626 bump PR was verified against. + +> **⚠ Historical record — the revs below are NOT the current pins.** +> This page is frozen at the state of the #2626 bump, which landed +> `amplihack-agent-eval` at `2a93441d…`. That pin has since moved twice: +> issue **#2767** advanced it to `14dc30b1…` (amplihack-rs PR #856, the clean +> agent-result channel; a 2026-07-07 UTC commit), and this change — verified +> **2026-09-03** — advanced it to `9ee05a06eab98e9ab504a031bffaa4190700c2af`, +> the amplihack-rs **v0.18.25** release source commit. `amplihack-memory` has +> likewise moved on (see +> [the WAL crash-consistency record](./cognitive-memory-wal-crash-consistency.md), +> issue #4687). The authoritative pins are always the live lines in the root +> `Cargo.toml`, guarded by `tests/issue_2626_amplihack_pin_bump.rs` (which rev +> is pinned) and `tests/amplihack_agent_eval_api_compat.rs` (that the pinned +> crate still exposes the API `src/gym_runner_client.rs` calls). The *method* +> this page documents — lockstep, API-parity, and supply-chain re-verification — +> remains the current procedure; only its rev table is historical. + +> **What is reusable here vs. what is obsolete.** Read this page with the split +> below; individual sections carry their own ⚠ markers. +> +> | Still current — the **method** | Superseded — the **target policy** | +> | --- | --- | +> | Pin by **immutable 40-char commit SHA** on the upstream default-branch line, never a branch/tag ref | "The pin must equal the **current** upstream `main` HEAD" | +> | **Manifest ↔ lockfile parity**: `Cargo.lock` records exactly the pinned revs | "Drift from `main` must be driven back to **zero**" | +> | **API-parity / compatibility** check before adopting a rev | "Bump whenever `main` has advanced" | +> | **`lbug` lockstep**: exactly one engine / one store format | | +> | **Supply-chain re-verification** (`cargo deny` / `audit` / `vet`) | | +> | **Build + test gates**, and the done-gate that the fix must run in Simard's own build | | +> +> Under the current policy a pin is adopted because it is an **approved target** +> — for `amplihack-agent-eval` the reviewed **`v0.18.25` release source commit** +> `9ee05a06…`. Upstream `main` moves past that commit continuously, and that +> drift does **not** make the pin invalid or stale. See +> [Drift is a signal, not an automatic bump](../howto/self-maintain-dependency-pins.md#drift-is-a-signal-not-an-automatic-bump). Two of the four git-rev pins in the root `Cargo.toml` had drifted behind their upstream default branch: @@ -37,17 +72,21 @@ upstream default branch: A git-rev pin is reproducible but **frozen**: until the pin moves, merged upstream work is absent from the daemon that depends on it. Per the operator policy — *when Simard updates a tool she maintains she must bump her **own** -dependency and run the new code* — this reconcile moves both pins to the exact -`main` HEADs and re-verifies the whole graph. +dependency and run the new code* — this reconcile **moved** both pins to what +were then the exact `main` HEADs and re-verified the whole graph. (Choosing +`main` HEAD as the target was the #2626-era policy; see the banner above for the +current target rule.) --- ## What changed -Both pins were re-pointed to the exact 40-character `main` HEAD of their -upstream repository. No other dependency, feature, or profile was touched. +Both pins were re-pointed to what was, **at #2626 bump time**, the exact +40-character `main` HEAD of their upstream repository — that was the approved +target then; it is not a standing rule (see the banner above). No other +dependency, feature, or profile was touched. -| Crate | Upstream repo | Old rev (behind `main`) | New rev (`main` HEAD) | +| Crate | Upstream repo | Old rev (behind `main`) | New rev (`main` HEAD *at the time*) | | --- | --- | --- | --- | | `amplihack-agent-eval` | `rysweet/amplihack-rs` | `59548a96049ab8d558110bcaf9c82a4316f1bbf0` | `2a93441d1837f9f853d5dddc56cc1088353a8872` | | `amplihack-memory` | `rysweet/amplihack-memory-lib` | `5d7db77dd5c3bafb2846c2f50761112588a47563` | `f80037089a735bd0d394e3eec5cea9fcae1895ea` | @@ -66,18 +105,28 @@ lbug = "=0.17.1" --- -## Provenance: both revs are upstream `main` HEADs +## Provenance: both revs were upstream `main` HEADs *at #2626 bump time* + +> **⚠ Historical — obsolete target policy.** This section records the #2626-era +> rule that a pin must equal the **current** upstream `main` HEAD. That rule is +> **superseded**. `amplihack-agent-eval` is now pinned to a reviewed **release** +> commit (`v0.18.25`), and upstream `main` advances past any pin continuously, so +> "equals `main` HEAD" is no longer a validity condition. What remains current is +> the narrower requirement below: a pin must be an **immutable commit SHA on the +> upstream default-branch line**, never a feature-branch ref. See +> [Drift is a signal, not an automatic bump](../howto/self-maintain-dependency-pins.md#drift-is-a-signal-not-an-automatic-bump). Each new rev was taken from the live upstream default branch at bump time, not from a feature branch (a feature-branch ref can be force-pushed or GC'd, which -would freeze the build against an unmergeable commit): +would freeze the build against an unmergeable commit) — **that** constraint is +still in force today: ```bash git ls-remote https://github.com/rysweet/amplihack-rs.git main -# 2a93441d1837f9f853d5dddc56cc1088353a8872 refs/heads/main +# 2a93441d1837f9f853d5dddc56cc1088353a8872 refs/heads/main # as of 2026-06/07 git ls-remote https://github.com/rysweet/amplihack-memory-lib.git main -# f80037089a735bd0d394e3eec5cea9fcae1895ea refs/heads/main +# f80037089a735bd0d394e3eec5cea9fcae1895ea refs/heads/main # as of 2026-06/07 ``` After the bump, `Cargo.lock` records the identical revs for both crates. Confirm @@ -92,12 +141,22 @@ grep -A3 'name = "amplihack-agent-eval"' Cargo.lock # source ... #2a93441... grep -A3 'name = "amplihack-memory"' Cargo.lock # source ... #f800370... ``` -The drift that motivated the bump is verifiable with the GitHub compare API -against the *old* rev (both report `behind_by > 0` before the bump, `0` after): +The drift that motivated the #2626 bump was measured with the GitHub compare API +against the *old* revs. + +> **⚠ Two defects in the original snippet, preserved here only as history.** +> (1) **Wrong field.** With `base=` and `head=main`, `ahead_by` is how far +> `main` is ahead of the pin; `behind_by` is **always `0`** in this orientation, +> so the original `--jq '.behind_by'` printed `0` regardless of drift. +> (2) **Obsolete criterion.** "Drift must return to `0`" is no longer a +> correctness condition — see the corrected, current procedure in +> [Trigger B](../howto/self-maintain-dependency-pins.md#trigger-b--the-proactive-reconcile). +> The corrected form of the measurement is: ```bash -gh api repos/rysweet/amplihack-rs/compare/59548a96049ab8d558110bcaf9c82a4316f1bbf0...main --jq '.behind_by' -gh api repos/rysweet/amplihack-memory-lib/compare/5d7db77dd5c3bafb2846c2f50761112588a47563...main --jq '.behind_by' +# base=, head=main => .ahead_by = commits main is ahead of the pin +gh api repos/rysweet/amplihack-rs/compare/59548a96049ab8d558110bcaf9c82a4316f1bbf0...main --jq '.ahead_by' +gh api repos/rysweet/amplihack-memory-lib/compare/5d7db77dd5c3bafb2846c2f50761112588a47563...main --jq '.ahead_by' ``` --- @@ -195,9 +254,17 @@ cargo vet --locked # transitive trust certification already-allowlisted remotes (`amplihack-rs.git`, `amplihack-memory-lib.git`). `unknown-git = "deny"` holds; the bump never adds a git source to work around the allowlist. -- **Pin integrity.** Each pin is an exact 40-char SHA verified equal to its - upstream `main` HEAD, and `Cargo.lock`'s git rev is re-confirmed to match after - `cargo update`. +- **Pin integrity.** *(Current rule — not #2626-specific.)* Each pin must be an + **immutable full 40-char commit SHA**, verified equal to the **approved target + commit** chosen for that bump, with **manifest/lock parity**: `Cargo.lock`'s + git rev is re-confirmed to match `Cargo.toml` after `cargo update`. The + *approved target* is whatever commit was reviewed and selected — at #2626 that + happened to be the upstream `main` HEAD, whereas current policy may select a + reviewed **release** commit instead (today `amplihack-agent-eval` targets the + `v0.18.25` release source commit `9ee05a06…`). Equality with the **moving** + `main` branch is therefore **not** a standing requirement, and `main` advancing + past a pin does not breach pin integrity — see + [Drift is a signal, not an automatic bump](../howto/self-maintain-dependency-pins.md#drift-is-a-signal-not-an-automatic-bump). - **No new transitive crates.** `amplihack-agent-eval` stays light (`serde`/`serde_json`/`thiserror`/`tracing`/`chrono` only); the `amplihack-memory-lib` delta between `5d7db77` and `f800370` introduces no new @@ -234,9 +301,17 @@ Simard change does. ## Done-gate: the fix must run in Simard's own build Under the [dependency-pin reconcile](../howto/self-maintain-dependency-pins.md), -bumping the upstream repo is **not** "done". This reconcile is done only once: +bumping the upstream repo is **not** "done". This reconcile was done only once: -1. Both `Cargo.toml` revs equal the upstream `main` HEADs. +> **⚠ Criterion 1 is #2626-era history.** "Revs equal the upstream `main` HEADs" +> encodes the superseded track-current-`main` target policy. The **current** +> form of criterion 1 is: *both `Cargo.toml` revs equal the immutable commit +> SHAs of the **approved targets** chosen for this bump* — for +> `amplihack-agent-eval` today that is the `v0.18.25` release source commit, not +> whatever `main` points at. **Criteria 2–6 are unchanged and still current.** + +1. ~~Both `Cargo.toml` revs equal the upstream `main` HEADs.~~ → *now:* both + `Cargo.toml` revs equal the approved target commits for the bump. 2. `Cargo.lock` records those same revs. 3. `cargo build --release` and `cargo test` pass. 4. `cargo tree -i lbug` shows a single `0.17.1`. @@ -251,15 +326,25 @@ this reconcile's goal to report done. ## Reproduce / verify end-to-end +> **⚠ Step 1 is #2626-era history and must not be run as a current gate.** It +> asserts zero drift from `main`, which is the superseded target policy, and it +> reads `.behind_by` in an orientation where that field is always `0` — so it +> would appear to "pass" unconditionally. Steps 2–4 are the **reusable method** +> and remain current, unchanged. + ```bash -# 1. Both pins point at the current upstream main HEAD (0 drift): +# 1. HISTORICAL (#2626 target policy — do NOT use as a current gate): +# "both pins equal the current upstream main HEAD (0 drift)". +# CURRENT equivalent: assert each pin equals its approved TARGET commit, +# and treat drift from main as informational only: for pair in \ "amplihack-rs:amplihack-agent-eval" \ "amplihack-memory-lib:amplihack-memory"; do repo=${pair%%:*}; crate=${pair##*:} - pinned=$(grep "$crate " Cargo.toml | grep -oE '[0-9a-f]{40}') - behind=$(gh api "repos/rysweet/$repo/compare/$pinned...main" --jq '.behind_by') - echo "$crate: pin=$pinned behind_by=$behind" # behind_by must be 0 + pinned=$(sed -n "s/^$crate[[:space:]]*=.*,[[:space:]]*rev[[:space:]]*=[[:space:]]*\"\([0-9a-f]\{40\}\)\".*/\1/p" Cargo.toml) + # base=pin, head=main => .ahead_by (main ahead of pin). NOT .behind_by. + ahead=$(gh api "repos/rysweet/$repo/compare/$pinned...main" --jq '.ahead_by') + echo "$crate: pin=$pinned main_ahead_of_pin=$ahead # informational, need not be 0" done # 2. Exactly one engine: diff --git a/docs/reference/cognitive-memory-wal-crash-consistency.md b/docs/reference/cognitive-memory-wal-crash-consistency.md index 5d561ae34..448d20ef1 100644 --- a/docs/reference/cognitive-memory-wal-crash-consistency.md +++ b/docs/reference/cognitive-memory-wal-crash-consistency.md @@ -1,7 +1,7 @@ --- title: Cognitive-memory WAL crash-consistency and single-owner checkpointing (#4687) description: "Reference for the cognitive-memory write-ahead-log durability fix — single-owner checkpointing (the engine's own auto-checkpoint disabled on the read-write path), fsync-durable clean shutdown, fsync-before-advance checkpoint ordering, and the explicit error-level + monotonic-counter observability that replaces the previously silent good-prefix tail truncation. Additive and non-breaking; store format stays v42." -last_updated: 2026-07-25 +last_updated: 2026-09-03 review_schedule: as-needed owner: simard doc_type: reference @@ -194,7 +194,8 @@ auto-checkpoint is disabled (see [Operational notes](#operational-notes)). wrapper. The store and WAL remain readable — no sidecar files or format changes were introduced. - **Engine lockstep:** the fix is wrapper-only, so the `lbug` engine rev - (`rysweet/ladybug-rust`, `Cargo.toml:158`) is **unchanged**. Exactly one + (`rysweet/ladybug-rust`, the `lbug = { git = …, rev = … }` key in + `[dependencies]`) is **unchanged**. Exactly one `lbug` version stays linked (`cargo tree -p lbug` shows one line). The engine rev bumps **only** if a defect is proven to live in engine code, in which case the memory-lib engine pin and Simard's direct `lbug` pin move together. @@ -207,7 +208,7 @@ Simard consumes the fix by bumping the `amplihack-memory` git rev in tag `issue-4687-wal-crash-consistency-c266e15`), then refreshing `Cargo.lock`: ```bash -# Cargo.toml:135 — amplihack-memory rev → 0031505b911151bf47409694a6c45f8b778d91b9 +# Cargo.toml `amplihack-memory` key — rev → 0031505b911151bf47409694a6c45f8b778d91b9 cargo update -p amplihack-memory # refreshes Cargo.lock cargo tree -p lbug # must show exactly one lbug version ``` @@ -226,7 +227,7 @@ cargo tree -p lbug # must show exactly one lbug version > **Lockstep caveat.** The fix is wrapper-only, so the merge commit keeps > amplihack-memory's own `lbug` pin at the current fork rev (`5a2c1078…`). If > `cargo tree -p lbug` ever reports **two** versions after the bump, Simard's -> direct `lbug` dep (`Cargo.toml:158`) must be moved to the **same** rev in the +> direct `lbug` dep (the `lbug = …` key in `[dependencies]`) must be moved to the **same** rev in the > same PR to restore the one-engine invariant and avoid the `std::format` ABI > SIGSEGV. diff --git a/docs/reference/rpc-wire-protocol.md b/docs/reference/rpc-wire-protocol.md index be80f3e99..6fc843ae1 100644 --- a/docs/reference/rpc-wire-protocol.md +++ b/docs/reference/rpc-wire-protocol.md @@ -1,7 +1,7 @@ --- title: RPC Wire Protocol Reference description: Complete JSON-RPC-style wire protocol specification for all Simard RPC methods. -last_updated: 2026-07-06 +last_updated: 2026-09-03 owner: simard doc_type: reference --- @@ -524,7 +524,7 @@ part of the suite; run it on its own via `gym.run_scenario`. | Field | Type | Description | |-------|------|-------------| | suite_id | string | Echoes the requested label | -| success | bool | `true` only if **every** scenario passed (`scenarios_passed == scenarios_total`). The adapter computes this directly from the per-scenario results; it does **not** trust the engine's suite-level flag, which has a known inverted-logic quirk upstream. | +| success | bool | `true` only if **every** scenario passed (`scenarios_passed == scenarios_total`). The adapter computes this directly from the per-scenario results; it does **not** trust the engine's suite-level flag, whose upstream expression is a tautology that always evaluates to `true` and therefore carries no information. | | overall_score | float | Mean score across passing scenarios, in `[0.0, 1.0]` | | dimensions | object | Five-key aggregate (mean across passing scenarios) | | scenario_results | array | One `gym.run_scenario`-shaped object per scenario; each `scenario_id` is the advertised descriptive id, restored by the adapter | diff --git a/prompt_assets/simard/engineer_system.md b/prompt_assets/simard/engineer_system.md index 6712855b4..8308a8607 100644 --- a/prompt_assets/simard/engineer_system.md +++ b/prompt_assets/simard/engineer_system.md @@ -326,8 +326,10 @@ Simard's root `Cargo.toml` pins the tools she maintains by **exact git rev**: done when the upstream PR merges** — follow through in the **same cycle** and **bump your own pin**: -1. Edit the matching `rev = ...` line in the root **`Cargo.toml`** to the merged - upstream `main` commit SHA. +1. Edit the matching `rev = ...` line in the root **`Cargo.toml`** to the + **exact immutable SHA of the merge commit that carried your change** — that + specific commit is the approved target here, **not** "whatever upstream + `main` points at now" (it may already have moved on). 2. Re-verify with **`cargo build`** (use the low-space variant `scripts/cargo-low-space build` when disk is tight). A bump that does **not** build is rolled back, not shipped. @@ -360,9 +362,23 @@ never preempts an active goal), watch for **dependency-drift**: a pinned rev tha has **fallen behind** its upstream default branch. Detect it with runtime git tooling — no new Rust subsystem — e.g. `git ls-remote https://github.com//.git main` compared against the -pinned rev (or `gh api repos///compare/...main --jq .behind_by`). -When a pin has drifted, open or update the same **bump PR** as above to re-point -the rev, `cargo build`-verify, and land it. Full reference: +pinned rev (or `gh api repos///compare/...main --jq .ahead_by` +— with `base=` and `head=main`, `.ahead_by` is how far **main is ahead of +the pin**; `.behind_by` is always `0` in this orientation and reading it is a +silent no-op). + +Drift is a **signal, not an automatic bump**. Simard pins immutable commit SHAs, +and some pins deliberately target a reviewed **release** commit, so a non-zero +count is normal and expected as upstream keeps committing. Open or update the +same **bump PR** as above only when a newer **approved target** should be adopted +(a newer reviewed release tag, or the specific merged commit carrying a wanted +fix) — re-point the rev to that exact commit, `cargo build`-verify, and land it. +Never chase whatever `main` happens to be at that moment. + +Judge urgency by **what the drift contains**, not by how far it stretches. A +security advisory or a fix Simard actually requires is a real reason to select a +new target and can warrant normal (not idle-only) priority; a large commit count +with nothing Simard needs is merely informational. Full reference: `docs/howto/self-maintain-dependency-pins.md`. ### Allowed exceptions (must be recorded in `cycle_summary.engineer_summary`) diff --git a/prompt_assets/simard/goal_session_objective.md b/prompt_assets/simard/goal_session_objective.md index 66842b232..3c80a9e7e 100644 --- a/prompt_assets/simard/goal_session_objective.md +++ b/prompt_assets/simard/goal_session_objective.md @@ -185,7 +185,9 @@ merging it upstream. The goal **is not done until** that fix is shipped into Simard's own **running build**: 1. **Bump her own pin.** Edit the matching `rev = ...` line in the root - `Cargo.toml` to the merged upstream `main` commit. + `Cargo.toml` to the **exact immutable SHA of the merge commit that carried + that fix** — that specific commit is the approved target, **not** "whatever + upstream `main` points at now". 2. **Verify the build.** `cargo build` must succeed against the new rev; a bump that does not build is rolled back, not shipped. 3. **Land the bump PR.** Open (or update) a **bump PR** against `rysweet/Simard` @@ -394,11 +396,26 @@ require a subsequent rebuild cycle. The same "detect drift, reconcile when idle" posture applies to the upstream repos Simard **pins**, not just the Simard repo itself. As **low-priority self-maintenance** that fits spare "ok to be idle" research time (and never -preempts an active goal), periodically check whether any rev-pinned -build-dependency has **fallen behind** its upstream default branch; if so, open -or update a bump follow-up that re-points the rev, runs `cargo build`, and lands -it through the normal pipeline. This **dependency-drift** reconcile is the -upstream-repo analog of the Self-update awareness above. +preempts an active goal), periodically measure whether any rev-pinned +build-dependency has **fallen behind** its upstream default branch. + +Drift is a **signal for evaluation, not an automatic bump or follow-up.** Simard +pins immutable commit SHAs, and some pins deliberately target a reviewed +**release** commit, so a non-zero commit distance is normal and expected as +upstream keeps committing — by itself it does **not** mean the pin is stale and +does **not** justify opening a follow-up. Open or update a bump follow-up +**only** when a newer **approved target** has been selected: a newer reviewed +release tag, or the specific merged commit carrying a wanted fix. Then re-point +the rev to that exact commit, run `cargo build`, and land it through the normal +pipeline. Never chase whatever `main` happens to be at that moment. + +Judge urgency by **what the drift contains**, not by how far it stretches. A +security advisory or a fix Simard actually requires is a real reason to select a +new target and can warrant normal (not idle-only) priority; a large commit count +with nothing Simard needs is merely informational. This **dependency-drift** +reconcile is the upstream-repo analog of the Self-update awareness above; the +full rule lives in `engineer_system.md` and +`docs/howto/self-maintain-dependency-pins.md`. # Two response shapes diff --git a/src/gym_runner_client.rs b/src/gym_runner_client.rs index 4794e78f2..a58403938 100644 --- a/src/gym_runner_client.rs +++ b/src/gym_runner_client.rs @@ -101,10 +101,20 @@ fn sanitized(lookup: impl Fn(&str) -> Option) -> ScoreDimensions { /// Recompute the suite-level `success` flag from the per-scenario tallies. /// -/// The library's `GymRunner::run_suite` computes its top-level `success` with -/// inverted logic (it is `true` precisely when failures exist). The adapter -/// must NOT trust that flag — `success` is `true` only when every scenario in -/// the suite passed: `scenarios_passed == scenarios_total`. +/// The library's `GymRunner::run_suite` computes its top-level `success` as +/// `!result.failed_levels.is_empty() || result.level_results.iter().all(|lr| lr.success)`. +/// `ProgressiveResult::add_result` pushes a level id onto `failed_levels` +/// exactly when that level's result is NOT successful, so an empty +/// `failed_levels` implies every entry of `level_results` succeeded. One +/// disjunct or the other therefore holds in every reachable state: the +/// expression is a **tautology that always evaluates to `true`** — for an +/// all-pass suite, a partially-failing suite, and an empty suite alike +/// (`all()` over an empty iterator is `true`). +/// +/// So the flag is not merely "inverted"; it carries **no information at all**. +/// The adapter must therefore ignore it entirely and derive the verdict itself: +/// `success` is `true` only when every scenario in the suite passed, i.e. +/// `scenarios_passed == scenarios_total`. pub(crate) fn suite_success(scenarios_passed: usize, scenarios_total: usize) -> bool { scenarios_passed == scenarios_total } @@ -503,7 +513,7 @@ mod tests { assert_eq!(d.temporal_awareness, 0.0); } - // ── Suite success recompute (the upstream inverted-flag fix) ────────── + // ── Suite success recompute (upstream flag is an always-true tautology) ── #[test] fn run_suite_success_requires_all_passed() { @@ -513,7 +523,8 @@ mod tests { ); assert!( !suite_success(11, 12), - "a single failure => success: false (must not trust the engine's inverted flag)" + "a single failure => success: false (the engine's own flag is an \ + always-true tautology and must not be trusted)" ); assert!( !suite_success(0, 12), diff --git a/tests/amplihack_agent_eval_api_compat.rs b/tests/amplihack_agent_eval_api_compat.rs new file mode 100644 index 000000000..165993812 --- /dev/null +++ b/tests/amplihack_agent_eval_api_compat.rs @@ -0,0 +1,376 @@ +//! Compile-time + behavioural compatibility guard for the pinned +//! `amplihack-agent-eval` crate surface Simard's gym adapter consumes. +//! +//! # Why this exists +//! +//! `Cargo.toml` pins `amplihack-agent-eval` to the amplihack-rs **v0.18.25** +//! release source commit `9ee05a06eab98e9ab504a031bffaa4190700c2af` (annotated +//! tag `v0.18.25` → `e947170a…`, which dereferences to that commit; it was also +//! `refs/heads/main` when verified on **2026-09-03**). The previous pin, +//! `14dc30b1…` (landed by issue #2767), was a 2026-07-07 UTC commit 200 commits +//! behind. +//! +//! `tests/issue_2626_amplihack_pin_bump.rs` guards *which* rev is pinned by +//! reading `Cargo.toml` / `Cargo.lock` as text. It deliberately never links the +//! crate, so it cannot tell whether the pinned code still *has* the API +//! `src/gym_runner_client.rs` calls. This file is the other half of that +//! contract: it **links the real pinned crate** and fails to compile if the +//! consumed surface drifts in any way that matters to Simard. +//! +//! # What "the consumed surface" is +//! +//! `src/gym_runner_client.rs` — the only Simard module that imports the crate — +//! uses exactly: +//! +//! * `GymConfig { output_dir, agent_name, sdk, grader_votes }` (struct-literal +//! construction, so every field must exist with the right type *and* no +//! field may be added); +//! * `GymRunner::new(GymConfig) -> GymRunner`; +//! * `GymRunner::list_scenarios(&self) -> Vec` and `GymScenario::id`; +//! * `GymRunner::run_scenario(&self, &str) -> Result`; +//! * `GymRunner::run_suite(&self, &str) -> Result`; +//! * the `GymScenarioResult` / `GymSuiteResult` **types**, whose full field +//! set Simard compiles against even though the adapter deliberately does +//! not forward every field (see below). +//! +//! # Field tests are exhaustive, but do not imply every field is forwarded +//! +//! The destructuring tests assert **field-set compatibility**: the exact shape +//! Simard compiles against. They are written without `..` on purpose, so an +//! upstream field that is *added*, *removed*, *renamed*, or *retyped* is a +//! compile error. That is a stronger and different property than "the adapter +//! reads this field" — an upstream field Simard ignores today still changes the +//! type Simard links against, and a silently-absorbed new field is exactly the +//! drift this file exists to catch. +//! +//! Three destructured fields are, by design, **not** forwarded to the wire: +//! +//! * `GymScenarioResult::scenario_id` — read only as a *lookup key* +//! (`compact_id_map.get(&sr.scenario_id)`). The wire `scenario_id` is +//! intentionally **replaced** by the adapter's `wire_id`, so the engine's +//! bare `"L{n}"` form is normalized to the descriptive `"L{n}-{slug}"` id +//! that `gym.list_scenarios` advertises. +//! * `GymSuiteResult::suite_id` — **not consumed**; the handler echoes the +//! caller's requested `suite_id` instead. +//! * `GymSuiteResult::success` — **not consumed**; the adapter *recomputes* +//! it as `scenarios_passed == scenarios_total`. The library's own flag is +//! `!failed_levels.is_empty() || level_results.iter().all(|lr| lr.success)`, +//! and because `ProgressiveResult::add_result` records a level in +//! `failed_levels` exactly when it did *not* succeed, an empty +//! `failed_levels` implies every level succeeded. One disjunct always +//! holds, so the expression is a **tautology that is always `true`** — on +//! all-pass, partially-failing, and empty suites alike. It carries no +//! information whatsoever, so it is ignored rather than corrected. +//! +//! Every other destructured field is forwarded: `GymScenario` is serialized +//! whole by `gym.list_scenarios`, and the remaining `GymScenarioResult` / +//! `GymSuiteResult` fields are mapped onto the wire JSON. +//! +//! # How drift is detected +//! +//! * **Exhaustive destructuring** (`let Type { a, b, .. } = ..` is *not* used — +//! patterns are written without `..`) makes an *added or removed* upstream +//! field a compile error, not a silently ignored one. +//! * **Function-pointer coercion** to a written-out `fn(...) -> ...` type makes +//! any signature/return-type change a compile error. +//! * Two **behavioural** tests call the real pinned engine: `list_scenarios` +//! must still advertise the scenario ids `compact_id_map` splits on, and +//! `run_scenario` must still reject an unknown id as `Err(EvalError)` (the +//! adapter turns that `Err` into a structured failing result, so the error +//! path is part of the consumed contract). +//! +//! # What this file deliberately does NOT cover +//! +//! It does **not** exercise the adapter's wire-JSON mapping. Those helpers +//! (`scenario_value`, `dims_value`, `suite_success`, `fail_*` / `skip_*`) are +//! `pub(crate)` / private and are covered by the in-crate unit tests in +//! `src/gym_runner_client.rs` (e.g. `run_suite_success_requires_all_passed`) +//! plus the `tests/gym_eval.rs` handler tests. This file's job is narrower and +//! complementary: prove the *pinned upstream crate* still offers the surface +//! those tests depend on. +//! +//! No network, no LLM, no subprocess, no filesystem writes: `list_scenarios` is +//! pure, `run_scenario` returns the unknown-id `Err` before it builds a grader +//! or touches `output_dir`, and every result value is constructed locally. + +use std::collections::HashMap; +use std::path::PathBuf; + +use amplihack_agent_eval::error::EvalError; +use amplihack_agent_eval::gym::{ + GymConfig, GymRunner, GymScenario, GymScenarioResult, GymSuiteResult, +}; + +// ───────────────────────────────────────────────────────────────────────────── +// Compile-time surface guards +// ───────────────────────────────────────────────────────────────────────────── + +/// `GymRunner`'s three methods must keep the exact signatures the adapter calls. +/// +/// Coercing each method to a written-out `fn` type is a compile error the moment +/// a parameter, receiver, or return type changes (e.g. `run_scenario` becoming +/// `async`, taking `&mut self`, or returning a different error type). +#[test] +fn gym_runner_method_signatures_are_unchanged() { + let _new: fn(GymConfig) -> GymRunner = GymRunner::new; + let _list: fn(&GymRunner) -> Vec = GymRunner::list_scenarios; + let _run_scenario: fn(&GymRunner, &str) -> Result = + GymRunner::run_scenario; + let _run_suite: fn(&GymRunner, &str) -> Result = + GymRunner::run_suite; +} + +/// `GymConfig` must keep exactly the four public fields the adapter sets. +/// +/// The struct literal fails to compile if a field is removed or retyped; the +/// field-list-exhaustive destructuring (no `..`) fails if upstream *adds* one, +/// which would silently change the config the adapter builds. +#[test] +fn gym_config_has_exactly_the_fields_the_adapter_sets() { + let config = GymConfig { + output_dir: PathBuf::from("target/simard-gym").join("eval"), + agent_name: "simard-gym-eval".to_string(), + sdk: "mini".to_string(), + grader_votes: 3, + }; + + let GymConfig { + output_dir, + agent_name, + sdk, + grader_votes, + } = config; + + let _: PathBuf = output_dir; + let _: String = agent_name; + let _: String = sdk; + let _: u8 = grader_votes; +} + +/// `GymScenarioResult` must keep exactly this field set. +/// +/// Exhaustive by design — this asserts the *shape* Simard compiles against, not +/// that the adapter forwards each field. `scenario_id` in particular is read +/// only as a `compact_id_map` lookup key; the wire `scenario_id` is +/// intentionally replaced by the adapter's `wire_id`. +/// +/// `dimensions` must stay `HashMap>` — the adapter's +/// `dimensions_from_optional` depends on the `Option` inner type to distinguish +/// "not measured" from "measured zero" (honest degradation). +#[test] +fn gym_scenario_result_field_set_is_unchanged() { + let result = GymScenarioResult { + scenario_id: "L1".to_string(), + success: true, + score: 0.5, + dimensions: HashMap::from([("recall".to_string(), Some(0.5))]), + question_count: 4, + questions_answered: 2, + error_message: None, + degraded_sources: vec!["src".to_string()], + }; + + let GymScenarioResult { + scenario_id, + success, + score, + dimensions, + question_count, + questions_answered, + error_message, + degraded_sources, + } = result; + + let _: String = scenario_id; + let _: bool = success; + let _: f64 = score; + let _: HashMap> = dimensions; + let _: usize = question_count; + let _: usize = questions_answered; + let _: Option = error_message; + let _: Vec = degraded_sources; +} + +/// `GymSuiteResult` must keep exactly this field set. +/// +/// Exhaustive by design, and two of these fields are deliberately **not** +/// consumed by the `gym.run_suite` handler: `suite_id` (the handler echoes the +/// caller's requested id) and `success` (recomputed as +/// `scenarios_passed == scenarios_total`, because the library's own flag is a +/// tautology that always evaluates to `true` — see `suite_success` in +/// `src/gym_runner_client.rs` for the derivation — and so carries no +/// information). They are still destructured so an upstream change to either is +/// a compile error rather than a silent shape drift. +/// +/// Note `dimensions` here is `HashMap` (NOT `Option`) — the +/// adapter uses a different converter (`dimensions_from_required`) for the +/// suite than for the scenario, so the two inner types must not converge. +#[test] +fn gym_suite_result_field_set_is_unchanged() { + let result = GymSuiteResult { + suite_id: "progressive".to_string(), + success: false, + overall_score: 0.25, + dimensions: HashMap::from([("recall".to_string(), 0.25)]), + scenario_results: Vec::new(), + scenarios_passed: 1, + scenarios_total: 2, + error_message: Some("boom".to_string()), + degraded_sources: Vec::new(), + }; + + let GymSuiteResult { + suite_id, + success, + overall_score, + dimensions, + scenario_results, + scenarios_passed, + scenarios_total, + error_message, + degraded_sources, + } = result; + + let _: String = suite_id; + let _: bool = success; + let _: f64 = overall_score; + let _: HashMap = dimensions; + let _: Vec = scenario_results; + let _: usize = scenarios_passed; + let _: usize = scenarios_total; + let _: Option = error_message; + let _: Vec = degraded_sources; +} + +/// `GymScenario` must keep exactly this field set. +/// +/// Here the whole struct *is* consumed: `gym.list_scenarios` serializes +/// `Vec` verbatim with serde, so every field below reaches the +/// wire, and `compact_id_map` additionally splits `id`. +#[test] +fn gym_scenario_field_set_is_unchanged() { + let scenario = GymScenario { + id: "L1-recall".to_string(), + name: "Recall".to_string(), + description: "desc".to_string(), + level: "L1".to_string(), + question_count: 4, + article_count: 2, + }; + + let GymScenario { + id, + name, + description, + level, + question_count, + article_count, + } = scenario; + + let _: String = id; + let _: String = name; + let _: String = description; + let _: String = level; + let _: usize = question_count; + let _: usize = article_count; +} + +// ───────────────────────────────────────────────────────────────────────────── +// Behavioural guards — the pinned engine still answers the adapter's calls +// ───────────────────────────────────────────────────────────────────────────── + +/// The adapter's exact `GymConfig` (see `gym_runner_client::gym_config`) must +/// still build a runner whose advertised scenario ids match what +/// `compact_id_map` assumes. `list_scenarios` is pure — no LLM, no network, no +/// subprocess, and it does not touch `output_dir`. +/// +/// The contract `compact_id_map` depends on: the progressive scenarios are +/// advertised as `"L{n}-{slug}"`, while `run_suite` reports their per-level +/// results under the bare `"L{n}"` form. The adapter rebuilds the descriptive +/// id by splitting the advertised id on the first `-`, so those `L{n}` prefixes +/// must be **unique** — a collision would silently drop a mapping and emit the +/// wrong descriptive id on the wire. The runner also always appends the +/// `"long-horizon-memory"` scenario, which `run_scenario` special-cases. +#[test] +fn pinned_runner_advertises_the_scenario_ids_the_adapter_maps() { + let runner = GymRunner::new(GymConfig { + output_dir: PathBuf::from("target/simard-gym").join("eval"), + agent_name: "simard-gym-eval".to_string(), + sdk: "mini".to_string(), + grader_votes: 3, + }); + + let scenarios = runner.list_scenarios(); + assert!( + !scenarios.is_empty(), + "pinned amplihack-agent-eval must still advertise at least one gym \ + scenario; `gym.list_scenarios` would otherwise return an empty list." + ); + + let ids: Vec<&str> = scenarios.iter().map(|s| s.id.as_str()).collect(); + assert!( + ids.contains(&"long-horizon-memory"), + "the runner must still advertise the `long-horizon-memory` scenario \ + that `run_scenario` special-cases. Advertised ids: {ids:?}" + ); + + // Every advertised id must produce a non-empty compact key. + for id in &ids { + let compact = id + .split('-') + .next() + .expect("split always yields at least one element"); + assert!( + !compact.is_empty(), + "scenario id `{id}` has an empty compact prefix; `compact_id_map` \ + would map suite results onto an empty key." + ); + } + + // The progressive `L{n}-…` ids must exist and have unique `L{n}` prefixes. + let level_prefixes: Vec<&str> = ids + .iter() + .filter_map(|id| id.split('-').next()) + .filter(|c| { + c.starts_with('L') && c.len() > 1 && c[1..].chars().all(|ch| ch.is_ascii_digit()) + }) + .collect(); + assert!( + !level_prefixes.is_empty(), + "the runner must still advertise progressive `L{{n}}-…` scenarios; \ + `compact_id_map` has nothing to map otherwise. Advertised ids: {ids:?}" + ); + let unique: std::collections::BTreeSet<&str> = level_prefixes.iter().copied().collect(); + assert_eq!( + unique.len(), + level_prefixes.len(), + "advertised `L{{n}}` compact prefixes must be unique so `compact_id_map` \ + cannot silently drop a mapping. Got {level_prefixes:?}" + ); +} + +/// `GymRunner::run_scenario` must still report an unknown scenario id as an +/// `Err(EvalError)` rather than panicking or inventing a success. The adapter +/// converts that `Err` into a *structured failing result* (`fail_scenario`), +/// never an RPC error, so the error path is part of the consumed contract. +#[test] +fn pinned_runner_rejects_an_unknown_scenario_id_as_an_error() { + let runner = GymRunner::new(GymConfig { + output_dir: PathBuf::from("target/simard-gym").join("eval"), + agent_name: "simard-gym-eval".to_string(), + sdk: "mini".to_string(), + grader_votes: 3, + }); + + let err = runner + .run_scenario("definitely-not-a-scenario") + .expect_err("an unknown scenario id must be an Err, not a success"); + + // The adapter puts the rendered error into the wire `error_message`, so it + // must render as non-empty text. + assert!( + !err.to_string().trim().is_empty(), + "EvalError must render a non-empty message; the adapter surfaces it as \ + the wire `error_message` for a failing gym.run_scenario." + ); +} diff --git a/tests/issue_2626_amplihack_pin_bump.rs b/tests/issue_2626_amplihack_pin_bump.rs index 2cf78d14c..fd08e3226 100644 --- a/tests/issue_2626_amplihack_pin_bump.rs +++ b/tests/issue_2626_amplihack_pin_bump.rs @@ -1,14 +1,27 @@ -//! Failing TDD acceptance tests for issue #2626 — bump Simard's pinned -//! amplihack dependencies to current upstream `main` (Step 7). +//! Pin guards originally added as failing TDD acceptance tests for issue #2626 +//! — "bump Simard's pinned amplihack dependencies to current upstream `main`" +//! (Step 7). The file has outlived that framing: it now guards whatever the +//! **approved target** rev is, which for `amplihack-agent-eval` is a reviewed +//! **release** commit rather than `main`'s HEAD. //! //! # Policy this encodes //! //! When Simard consumes `amplihack-rs` (`amplihack-agent-eval`) and -//! `amplihack-memory-lib` (`amplihack-memory`) as git-pinned dependencies, a -//! self-improvement bump means: point *her own* pins at current upstream -//! `main` and run the new code. The pins are advanced as upstream lands work: +//! `amplihack-memory-lib` (`amplihack-memory`) as git-pinned dependencies, each +//! pin must be an **immutable commit SHA equal to the approved target** chosen +//! for that bump, with `Cargo.lock` in parity. Tracking upstream `main`'s +//! moving HEAD was the #2626-era target policy and is **superseded**: `main` +//! advances past any pin continuously, and that drift alone does not make a pin +//! stale. See `docs/howto/self-maintain-dependency-pins.md` ("Drift is a +//! signal, not an automatic bump"). //! -//! * `amplihack-agent-eval` 59548a96… → **2a93441d…** (amplihack-rs main) +//! The pins are advanced as new approved targets are adopted: +//! +//! * `amplihack-agent-eval` 59548a96… → 2a93441d… (landed by issue #2626) +//! → 14dc30b1… (landed by issue #2767, amplihack-rs PR #856's clean +//! agent-result channel; a 2026-07-07 UTC commit) → **9ee05a06…** +//! (amplihack-rs **v0.18.25** release source commit — see the provenance +//! block on `AGENT_EVAL_TARGET_REV` below) //! * `amplihack-memory` 901f63ad… → **72c5ea1b…** (memory-lib main — //! the squash-merge of PR #126, which serves the ranked-recall graph term //! from a single bulk graph-adjacency scan per edge type instead of a @@ -17,8 +30,10 @@ //! store-format change; Simard consumes it to fix the "memory graph never //! loads" pathology, issue #40) //! -//! These are the exact 40-char SHAs verified against `git ls-remote … main` -//! at authoring time. +//! These are the exact 40-char SHAs verified against upstream with +//! `git ls-remote` (plus `git cat-file` for the annotated-tag dereference). +//! Each constant below records **what** was verified and **when**; the header +//! bullets are history, the constants are the live contract. //! //! # The lockstep invariant //! @@ -41,10 +56,31 @@ use std::collections::BTreeSet; use std::fs; use std::path::PathBuf; -// ── Target / stale pin constants (verified against upstream `main`) ────────── +// ── Approved-target / stale pin constants (each verified upstream) ────────── -/// amplihack-rs `main` HEAD carrying the `amplihack-agent-eval` crate to adopt. -const AGENT_EVAL_TARGET_REV: &str = "14dc30b10e87764120c6f2bae7f3630522c29e5d"; +/// amplihack-rs source commit carrying the `amplihack-agent-eval` crate to +/// adopt: the **v0.18.25** release commit. +/// +/// Upstream provenance, verified 2026-09-03 against +/// `https://github.com/rysweet/amplihack-rs.git`: +/// +/// * `git ls-remote … refs/tags/v0.18.25` → `e947170a…` (the **annotated +/// tag object**), and `refs/tags/v0.18.25^{}` → `9ee05a06…` (the commit it +/// dereferences to). The pin is the dereferenced *commit*, never the tag +/// object and never the tag name — a tag can be force-moved, a commit SHA +/// cannot. +/// * `git ls-remote … refs/heads/main` → `9ee05a06…`, i.e. the release tag +/// and upstream `main` were the same commit at verification time. +/// * commit date 2026-09-02; release v0.18.25 published 2026-09-02. +/// * `9ee05a06…` is 200 commits ahead of the previous pin `14dc30b1…`. +/// +/// API impact of the 200-commit delta on this crate: **none**. Both +/// `14dc30b1…:crates/amplihack-agent-eval` and +/// `9ee05a06…:crates/amplihack-agent-eval` resolve to the identical git tree +/// `1a635333f15fa2964e6d0ecae35b5a9625d59ee7`, so the consumed crate source is +/// byte-for-byte unchanged. See `tests/amplihack_agent_eval_api_compat.rs` for the +/// compile-time guard that Simard actually uses that surface. +const AGENT_EVAL_TARGET_REV: &str = "9ee05a06eab98e9ab504a031bffaa4190700c2af"; /// amplihack-memory-lib commit carrying the `amplihack-memory` crate. /// /// Superseded by issue #4687: originally the #2626 bump target @@ -56,7 +92,15 @@ const AGENT_EVAL_TARGET_REV: &str = "14dc30b10e87764120c6f2bae7f3630522c29e5d"; const MEMORY_TARGET_REV: &str = "0031505b911151bf47409694a6c45f8b778d91b9"; /// The stale revs the bump must move *off of* (anti-regression sentinels). +/// +/// `AGENT_EVAL_STALE_REV` is the original pre-#2626 rev. +/// `AGENT_EVAL_PREVIOUS_REV` is the **#2767** bump target `14dc30b1…` (NOT +/// #2626, whose target was `2a93441d…`) — a **2026-07-07 UTC** commit +/// (2026-07-07 01:47:01 UTC / 2026-07-06 18:47:01 -0700) that was +/// upstream `main` then and is 200 commits behind now. Regressing to either is +/// a stale-pin regression. const AGENT_EVAL_STALE_REV: &str = "59548a96049ab8d558110bcaf9c82a4316f1bbf0"; +const AGENT_EVAL_PREVIOUS_REV: &str = "14dc30b10e87764120c6f2bae7f3630522c29e5d"; const MEMORY_STALE_REV: &str = "72c5ea1bfcca7e6f3e314dfd99fbe4998378ffe8"; /// The only git remotes these two crates may resolve from. A bump must never @@ -64,6 +108,22 @@ const MEMORY_STALE_REV: &str = "72c5ea1bfcca7e6f3e314dfd99fbe4998378ffe8"; const AGENT_EVAL_REMOTE: &str = "https://github.com/rysweet/amplihack-rs.git"; const MEMORY_REMOTE: &str = "https://github.com/rysweet/amplihack-memory-lib.git"; +/// The crate version Cargo.lock must record for `amplihack-agent-eval` at +/// `AGENT_EVAL_TARGET_REV`. +/// +/// `crates/amplihack-agent-eval/Cargo.toml` sets `version.workspace = true`, and +/// the amplihack-rs workspace manifest at `9ee05a06…` declares `version = +/// "0.18.0"` (it was `0.11.1` at the previous pin `14dc30b1…`). Because the pin +/// is an immutable commit SHA, this version is deterministic — asserting it +/// proves the lockfile entry was genuinely re-resolved from the new tree rather +/// than hand-edited to swap only the `source` string. +/// +/// Deliberate, documented mismatch: the upstream **release tag** is `v0.18.25` +/// while the upstream **workspace crate version** is `0.18.0`. The tag tracks +/// the amplihack-rs release train, not this crate's semver. Simard consumes the +/// crate by rev, so the rev — not the version — is the authoritative pin. +const AGENT_EVAL_LOCKED_VERSION: &str = "0.18.0"; + /// Simard's direct `lbug` dep (simard-tui goal board) is a git dep on the /// rysweet/ladybug-rust fork (issue #3119: fixes the from-source duplicate-symbol /// link + the libstdc++ std::format ABI SIGSEGV). It must pin the SAME fork rev @@ -189,22 +249,25 @@ fn is_full_sha(rev: &str) -> bool { } // ───────────────────────────────────────────────────────────────────────────── -// Primary contract — Cargo.toml pins the two crates at current upstream main +// Primary contract — Cargo.toml pins the two crates at their APPROVED TARGET +// revs (an immutable commit SHA each; NOT "whatever `main` points at now") // ───────────────────────────────────────────────────────────────────────────── #[test] -fn cargo_toml_pins_amplihack_agent_eval_to_target_main_rev() { +fn cargo_toml_pins_amplihack_agent_eval_to_approved_target_rev() { let rev = dep_rev(&cargo_toml(), "amplihack-agent-eval") .expect("Cargo.toml must declare a git `amplihack-agent-eval` dependency with a `rev`"); assert_eq!( rev, AGENT_EVAL_TARGET_REV, - "amplihack-agent-eval must be pinned to amplihack-rs `main` HEAD \ - {AGENT_EVAL_TARGET_REV} (#2626 bump). Found `{rev}`." + "amplihack-agent-eval must be pinned to the amplihack-rs v0.18.25 \ + release source commit {AGENT_EVAL_TARGET_REV} (annotated tag \ + `v0.18.25` dereferences to it, and it was `main` HEAD when verified \ + on 2026-09-03). Found `{rev}`." ); } #[test] -fn cargo_toml_pins_amplihack_memory_to_target_main_rev() { +fn cargo_toml_pins_amplihack_memory_to_approved_target_rev() { let rev = dep_rev(&cargo_toml(), "amplihack-memory") .expect("Cargo.toml must declare a git `amplihack-memory` dependency with a `rev`"); assert_eq!( @@ -225,7 +288,13 @@ fn cargo_toml_moves_off_the_stale_amplihack_revs() { assert_ne!( agent_rev, AGENT_EVAL_STALE_REV, "amplihack-agent-eval is still on the STALE rev {AGENT_EVAL_STALE_REV}; \ - #2626 requires moving to {AGENT_EVAL_TARGET_REV}." + the pin must be {AGENT_EVAL_TARGET_REV} (v0.18.25)." + ); + assert_ne!( + agent_rev, AGENT_EVAL_PREVIOUS_REV, + "amplihack-agent-eval regressed to the PREVIOUS pin \ + {AGENT_EVAL_PREVIOUS_REV} (a 2026-07-07 UTC commit, 200 commits behind); \ + the pin must be {AGENT_EVAL_TARGET_REV} (v0.18.25)." ); assert_ne!( memory_rev, MEMORY_STALE_REV, @@ -259,6 +328,26 @@ fn amplihack_pins_are_full_sha_revs_not_floating_refs() { // Lockfile parity — Cargo.lock git sources must match the bumped pins // ───────────────────────────────────────────────────────────────────────────── +#[test] +fn cargo_lock_records_the_agent_eval_version_from_the_bumped_rev() { + // The lockfile must carry the version the *new* rev's manifest resolves to + // (workspace version 0.18.0), not the previous rev's 0.11.1. A lockfile + // whose `source` moved but whose `version` did not was not re-resolved. + let versions = distinct_locked_versions(&cargo_lock(), "amplihack-agent-eval"); + assert_eq!( + versions.len(), + 1, + "expected exactly one locked amplihack-agent-eval; found {versions:?}" + ); + let version = versions.iter().next().expect("one locked version"); + assert_eq!( + version, AGENT_EVAL_LOCKED_VERSION, + "Cargo.lock must record amplihack-agent-eval {AGENT_EVAL_LOCKED_VERSION} \ + (the workspace version at rev {AGENT_EVAL_TARGET_REV}). Found \ + `{version}` — re-run `cargo update -p amplihack-agent-eval`." + ); +} + #[test] fn cargo_lock_source_rev_matches_bumped_agent_eval_pin() { let source = locked_source(&cargo_lock(), "amplihack-agent-eval") @@ -274,6 +363,11 @@ fn cargo_lock_source_rev_matches_bumped_agent_eval_pin() { "Cargo.lock amplihack-agent-eval source still references the STALE rev \ {AGENT_EVAL_STALE_REV}; the lockfile was not refreshed." ); + assert!( + !source.contains(AGENT_EVAL_PREVIOUS_REV), + "Cargo.lock amplihack-agent-eval source still references the PREVIOUS \ + rev {AGENT_EVAL_PREVIOUS_REV}; the lockfile was not refreshed." + ); } #[test]