From c84b2fb70a164ebce03aa1599099a4b1caa7d83b Mon Sep 17 00:00:00 2001 From: Brian Krabach Date: Sat, 25 Apr 2026 07:44:02 -0700 Subject: [PATCH] =?UTF-8?q?docs(experts):=20update=20core-expert=20+=20ker?= =?UTF-8?q?nel-overview=20+=20=C2=A710=20for=20v1.4.x=20lifecycle=20and=20?= =?UTF-8?q?pre-merge=20gate?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Surface recent kernel changes (v1.4.0/v1.4.1) in the in-repo expert context so core-expert is current and CORE_DEVELOPMENT_PRINCIPLES §10 no longer contradicts release-mandate.md. agents/core-expert.md: - Rename RELEASE Mode "Post-Merge Gate" → "Pre-Merge Gate"; reframe trigger from "after a merge" to pre-merge; cite release-mandate.md as superseding §10 - New MODULE LIFECYCLE Mode covering mount() and on_session_ready() pairing, with no-timeout footgun, Python-only polyglot scope, failure isolation, dispatch ordering, fork semantics - Knowledge Base: add @core:CONTRACTS.md as authoritative cross-boundary reference (was missing); add @core:context/release-mandate.md as authoritative for the Pre-Merge Gate - Per-protocol contracts subdirectory clarified as deep-dive path, CONTRACTS.md as the canonical map context/kernel-overview.md: - "What the Kernel Provides" → new "Module Lifecycle" subsection naming mount() and on_session_ready() with one-line descriptions - "Module Protocols" → new "Module Lifecycle" subsection with signature blocks matching the existing protocol style; cites CONTRACTS.md as authoritative docs/CORE_DEVELOPMENT_PRINCIPLES.md §10: - Full rewrite to match the new pre-merge gate model - Title: "The Release Gate: Every Merge Gets a Release" → "The Release Gate: Merge Is Release" - Lists the five in-PR requirements (atomic version bump, Rust/Python symmetry, fresh wheel, E2E smoke test result, no [tool.uv.sources] overrides) - Documents who merges (core owner) and recovery via Incident Playbook - Cross-references release-mandate.md as canonical Verifies in-repo expert + dev docs against the policy codified by PR #64 (release-mandate pre-merge gate) and the lifecycle contract introduced by PR #63 (on_session_ready) and shipped via v1.4.1. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- agents/core-expert.md | 80 +++++++++++++++++++++++------ context/kernel-overview.md | 27 ++++++++++ docs/CORE_DEVELOPMENT_PRINCIPLES.md | 74 +++++++++++++++----------- 3 files changed, 133 insertions(+), 48 deletions(-) diff --git a/agents/core-expert.md b/agents/core-expert.md index 77da8a12..b2674f6f 100644 --- a/agents/core-expert.md +++ b/agents/core-expert.md @@ -47,23 +47,57 @@ Provide: - HookResult patterns and capabilities - Event lifecycle and canonical events -### RELEASE Mode (Post-Merge Gate) +### RELEASE Mode (Pre-Merge Gate) -**When to activate**: Any question about merging a PR, what to do after a merge, version bumping, tagging, publishing, or CI triggering +**When to activate**: Any question about merging a PR to amplifier-core, version bumping, tagging, publishing, the wheel build, CI triggering, or how a change reaches PyPI. + +> **Authoritative source:** `@core:context/release-mandate.md` §"Pre-Merge Gate: Proof of Release Readiness". That section **supersedes** `@core:docs/CORE_DEVELOPMENT_PRINCIPLES.md §10` where they conflict. The mandate is canonical; the principles doc is being brought into alignment. If you see them disagree, trust the mandate. + +The model has shifted: **merge is release.** The PR proves readiness; the merge button is the publish button. There is no longer a "post-merge release window" to be careful about — that window has been closed by elevating every gate into the PR itself. Provide: -- Hard reference to section 10 of CORE_DEVELOPMENT_PRINCIPLES.md — the release gate is non-negotiable -- Note: this applies specifically to amplifier-core (PyPI package), not all ecosystem repos -- The three files that must be bumped in sync: `pyproject.toml`, `crates/amplifier-core/Cargo.toml`, `bindings/python/Cargo.toml` -- The atomic script: `python scripts/bump_version.py X.Y.Z` -- **The E2E smoke test**: `./scripts/e2e-smoke-test.sh` must pass before tagging. It validates - the built wheel in an isolated Docker container with a real LLM session. This gate was added - after three incidents in v1.2.3/v1.2.4 where the wheel was broken but all unit/integration - tests passed. -- The tag push: `git tag vX.Y.Z && git push origin main --tags` -- Why: `v*` tag triggers `rust-core-wheels.yml` → PyPI publish; this is the only path to production -- **Incident recovery**: If a broken version reaches PyPI, see the Incident Playbook in - `context/release-mandate.md` — yank on PyPI, fix forward, never reuse a version number. + +- Hard reference to `@core:context/release-mandate.md` § Pre-Merge Gate. Non-negotiable. +- Scope: this rule applies **only to amplifier-core** (the sole PyPI-published repo). Modules, bundles, foundation, and apps install from git and are unaffected. +- The five in-PR requirements: + 1. **Atomic version bump** — `python scripts/bump_version.py X.Y.Z` updates all three version files in sync (`pyproject.toml`, `crates/amplifier-core/Cargo.toml`, `bindings/python/Cargo.toml`). Manual edits drift; the script is the only sanctioned path. + 2. **Rust/Python event-constant symmetry** — every kernel event constant must be defined in **both** `crates/amplifier-core/src/events.rs` and `python/amplifier_core/events.py`, with matching membership in both `ALL_EVENTS` lists. Enforced by `bindings/python/tests/test_event_constants.py`. This rule was elevated to a pre-merge gate after the PR #63 round-3 review, which surfaced the broader anti-pattern of "Python fallback shim until next wheel build" — the wheel build is part of the PR, so there is no "next" to defer to. The same symmetry expectation applies to capability names and protocol identifiers. + 3. **Freshly built wheel** — regenerated binding artifacts present in the PR (`maturin develop` locally; CI verifies via `maturin build`). Python imports from `amplifier_core._engine`, so a stale wheel means a stale Python surface. + 4. **E2E smoke test result** — `./scripts/e2e-smoke-test.sh` run on the branch, output posted in the PR. Validates the built wheel in an isolated Docker container with a real LLM session. The v1.2.3/v1.2.4 incidents proved that 549 passing unit tests don't catch a broken wheel; this gate exists because of that. Includes the pristine-import preflight (Step 1b) added in v1.4.1 to catch undeclared runtime deps before the CLI install pulls them transitively. + 5. **No `[tool.uv.sources]` git overrides for `amplifier-core`** in downstream repos. +- **Who merges:** the core owner — not the author, not a delegate, not a reviewer. The merge click is the release commit. `v*` tag is created and pushed as part of the merge; `rust-core-wheels.yml` then builds wheels and publishes to PyPI. +- **Incident recovery:** if a broken version reaches PyPI, follow the Incident Playbook in `@core:context/release-mandate.md` — yank on PyPI, fix forward, never reuse a version number. Yanking is the recovery layer; the pre-merge gate is the prevention layer. Both stay. + +### MODULE LIFECYCLE Mode (mount + on_session_ready) + +**When to activate**: Questions about module initialization order, when to use `mount()` vs `on_session_ready()`, cross-module wiring, capability discovery after composition, or why a module's setup code can't see another module's contributions. + +> **Authoritative source:** `@core:CONTRACTS.md` § "Module Lifecycle Methods". Cite it. Don't paraphrase its contract details — point readers there. + +The kernel exposes a two-phase lifecycle. Modules implement either or both as **module-level free functions** (no `self`): + +- **`async def mount(coordinator, config)` — Required.** Called once per module, in phase order, while the coordinator is **partially composed**. Earlier-phase modules are accessible; later-phase modules may not yet be present. Use this for the module's own setup: open clients, register capabilities, register cleanup callables. May return a zero-argument cleanup callable (sync or async) — the kernel awaits it at teardown in reverse registration order. + +- **`async def on_session_ready(coordinator) -> None` — Optional.** Called **after every module across every phase has finished `mount()`**, before `session:fork` is emitted. The coordinator is fully composed: every contributed tool, hook, and provider is registered. Use this for cross-module wiring — discovering peers' contributions, subscribing to hooks that only exist after another module mounts, or eliminating the dual-path "check in mount, fall back at request time" anti-pattern. + +**Critical contract details to surface in any answer:** + +- **No timeout (footgun).** The kernel enforces no timeout on `on_session_ready`. A hanging callback hangs the session. CONTRACTS.md documents this as a deliberate absence — adding a timeout later would be a breaking change. Modules must not block here. +- **Failure isolation.** A raised exception in one module's `on_session_ready` is caught, logged as a WARNING with `exc_info=True`, and **emits a `module:on_session_ready_failed` event** with payload `{"module_id": str, "error": str}`. Remaining modules' callbacks still run. Log-only failures are invisible to observability hooks; the event is the observable signal. +- **Dispatch ordering.** Sequential, in mount registration order (orchestrator → context → providers → tools → hooks; load order within a phase). Stable and guaranteed. Cross-module assumptions on this order are safe. +- **Polyglot scope.** `on_session_ready` is **Python-only**. WASM, gRPC, and native Rust modules do **not** participate in the wave. Polyglot modules needing post-composition behavior must defer to a request-time check or emit a custom event for Python modules to subscribe to. +- **Fork semantics.** Fires **once per session**. A child session created by `session:fork` runs its own independent mount wave and its own `on_session_ready` pass — the parent's callbacks do not re-fire for the child. +- **Cleanup from `on_session_ready`.** Return value is ignored. If `on_session_ready` allocates a resource needing teardown, register the cleanup directly via `coordinator.register_cleanup(...)`. + +**When to recommend which:** + +| Need | Use | +|------|-----| +| Open a client; register own capability; register cleanup | `mount()` | +| Read another module's registered tool/hook/provider | `on_session_ready()` | +| Subscribe to events that other modules contribute | `on_session_ready()` | +| Anything blocking or slow | Neither — defer to request time | +| Cross-language module needs post-composition wiring | Custom event, **not** `on_session_ready` | --- @@ -71,10 +105,22 @@ Provide: ### Core Documentation -### Kernel Overview (Primary Context) +#### Authoritative Cross-Boundary Contract (Primary Reference) + +@core:CONTRACTS.md + +The Rust↔Python type, trait/protocol, error, and lifecycle mapping. **This is the canonical source for the module lifecycle (`mount`, `on_session_ready`), the trait↔protocol mapping, the data-model mapping, and the rules for modifying shared types.** Read this before answering any question about protocols or the FFI boundary. + +#### Kernel Overview (Primary Context) @core:context/kernel-overview.md +#### Release Mandate (Authoritative for the Pre-Merge Gate) + +@core:context/release-mandate.md + +Supersedes `docs/CORE_DEVELOPMENT_PRINCIPLES.md §10` where they conflict. Includes the Pre-Merge Gate, the post-merge checklist (still applicable, now proven in-PR), and the Incident Playbook. + ### Repository Development Principles @core:docs/CORE_DEVELOPMENT_PRINCIPLES.md @@ -87,9 +133,9 @@ Key documents for deep reference: - @core:docs/HOOKS_API.md - Complete hooks system documentation - @core:docs/MODULE_SOURCE_PROTOCOL.md - How modules are loaded -### Contract Specifications (Primary Reference) +### Per-Protocol Contract Specifications -**Use these as authoritative sources for protocol details:** +**Use these for protocol-specific deep dives. For the cross-boundary mapping and lifecycle, use `@core:CONTRACTS.md` (above) instead.** @core:docs/contracts/ diff --git a/context/kernel-overview.md b/context/kernel-overview.md index e2e094b2..f3afc6c7 100644 --- a/context/kernel-overview.md +++ b/context/kernel-overview.md @@ -63,6 +63,13 @@ Infrastructure context carrying: - Hook protocol: `__call__(event, data) → HookResult` - ContextManager protocol: `add_message/get_messages_for_request/get_messages/set_messages/clear` +### Module Lifecycle +The kernel calls two lifecycle entry points on every module, in deterministic order: +- `mount(coordinator, config)` — **required.** Per-module setup while the coordinator is partially composed. May return a cleanup callable. +- `on_session_ready(coordinator)` — **optional, Python-only.** Per-session callback after every module across every phase has finished `mount()`. Use for cross-module wiring against the fully-composed coordinator. + +See `@core:CONTRACTS.md` § Module Lifecycle Methods for the authoritative contract — including dispatch ordering, failure isolation, the `module:on_session_ready_failed` event, fork semantics, and the explicit no-timeout rule. + ## What the Kernel Does NOT Provide - ❌ Which modules to use (app layer decides) @@ -150,6 +157,26 @@ async def set_messages(self, messages: list[dict[str, Any]]) -> None async def clear(self) -> None ``` +### Module Lifecycle +Both are module-level free functions — no `self`, no class. + +```python +async def mount( + coordinator, + config: dict[str, Any] | None = None, +) -> Callable[[], None | Awaitable[None]] | None +``` + +```python +async def on_session_ready(coordinator) -> None +``` + +`mount()` runs in phase order; the coordinator is partially composed. The optional return value is a zero-argument cleanup callable (sync or async) invoked at teardown in reverse registration order. + +`on_session_ready()` runs after **all** modules in **all** phases have completed `mount()`, before `session:fork` is emitted. Python-only; sequential dispatch in mount registration order; exceptions are isolated and emit `module:on_session_ready_failed`. **No timeout — a hanging callback hangs the session.** + +Authoritative source: `@core:CONTRACTS.md` § Module Lifecycle Methods. + ## For Module Developers When building a module: diff --git a/docs/CORE_DEVELOPMENT_PRINCIPLES.md b/docs/CORE_DEVELOPMENT_PRINCIPLES.md index 6400404f..d398b9b1 100644 --- a/docs/CORE_DEVELOPMENT_PRINCIPLES.md +++ b/docs/CORE_DEVELOPMENT_PRINCIPLES.md @@ -160,59 +160,71 @@ This guarantee does NOT mean we can't evolve. It means evolution is additive — --- -## 10. The Release Gate: Every Merge Gets a Release +## 10. The Release Gate: Merge Is Release -**Every PR merged to `amplifier-core` main MUST be immediately followed by a version bump, release commit, `v{version}` tag, and tag push. No exceptions.** +**For `amplifier-core` only, every change that ships in the wheel proves end-to-end release readiness *in the PR*. The merge button is the publish button. There is no post-merge release window.** -This rule exists because `amplifier-core` occupies a unique position in the ecosystem: it is the **only repo published to PyPI**. The failure mode is concrete and was observed in production: +This rule supersedes the older "every merge is followed by a version bump and tag push" model. The gates are unchanged in substance; they have been moved earlier — from "immediately after merge" to "proven in the PR before merge." This eliminates the version-skew window between git HEAD and PyPI, removes the class of post-merge mistakes that produced the v1.2.3 and v1.2.4 incidents, and removes the temptation to land "Python fallback shim until next wheel build" intermediate states (the anti-pattern that surfaced in PR #63's round-3 review). -- Users install `amplifier-core` from PyPI and get a pinned version (e.g., v1.0.7). -- Downstream modules (`amplifier-module-*`, provider repos) install from git and track `main` directly. -- A PR merges to `main` that changes the API. No release is cut. PyPI still serves v1.0.7. -- Any user who installs or updates a module that tracks the new API now has a version skew. It breaks silently or with a confusing error. - -**This happened.** Commit `580ecc0` ("eliminate Python RetryConfig") merged on March 3, 2026 without a release. `provider-anthropic` was updated to use `initial_delay` instead of `min_delay`. All v1.0.7 PyPI users broke immediately. An emergency v1.0.8 hotfix was required. +> **Authoritative reference:** `context/release-mandate.md` § "Pre-Merge Gate: Proof of Release Readiness". That document is canonical; this section is its in-tree summary. The Incident Playbook (recovery via PyPI yank, fix-forward, never reuse a version number) lives there as well. ### Scope: amplifier-core Only -This rule applies **specifically to amplifier-core** because of its PyPI distribution. Other ecosystem repos — `amplifier-module-*`, `amplifier-bundle-*`, `amplifier-app-*`, provider repos — currently use `git+https` references for Python. Individual repo authors choose their own release process for those repos. Do not apply this mandate to them. +This rule applies **specifically to `amplifier-core`** because of its PyPI distribution. Other ecosystem repos — `amplifier-module-*`, `amplifier-bundle-*`, `amplifier-app-*`, `amplifier-foundation`, provider repos — install from git and pick up `main` directly. They use ordinary git-based workflows; individual repo authors choose their own release cadence. Do not apply this mandate to them. -### The Release Checklist (Every Merge) +### What the PR Must Include -1. **Determine the version increment** (semver rules): - - PATCH (`X.Y.Z+1`) — bug fixes, no API changes - - MINOR (`X.Y+1.0`) — additive API additions (new fields, new methods, backward compatible) - - MAJOR (`X+1.0.0`) — breaking API changes (removed fields, changed signatures) +Every PR to `amplifier-core` that changes code shipped in the wheel must include all of the following before the core owner clicks merge: -2. **Bump all three version files atomically** using the script: +1. **Atomic version bump.** Run: ```bash python scripts/bump_version.py X.Y.Z ``` - This updates in sync: - - `pyproject.toml` (line 3) - - `crates/amplifier-core/Cargo.toml` (line 3) - - `bindings/python/Cargo.toml` (line 3) + The script updates all three version files in sync — `pyproject.toml`, `crates/amplifier-core/Cargo.toml`, `bindings/python/Cargo.toml` — and warns if any were already out of sync (a canary for prior manual edits). Manual edits to individual files are forbidden; they are how v1.2.4's "version files not bumped before tagging" incident happened. Semver applies: PATCH for bug fixes, MINOR for additive API, MAJOR for breaking changes. + +2. **Rust/Python symmetry for kernel primitives.** Any new event constant, capability name, or protocol identifier must be defined in **both** sides — Rust (`crates/amplifier-core/src/events.rs`, etc.) and Python (`python/amplifier_core/events.py`, etc.) — with matching membership in both `ALL_EVENTS` lists. Enforced by `bindings/python/tests/test_event_constants.py`. The Python side imports from the Rust binding via `amplifier_core._engine`; there is no "Python fallback shim until next wheel build" path, because the wheel build is part of the PR. + +3. **Freshly built wheel.** The PR must include any regenerated binding artifacts. Run `maturin develop` locally; CI verifies via `maturin build`. A stale wheel means a stale Python surface, which means switchover tests pass but real users break. -3. **Run the E2E smoke test** before tagging: +4. **E2E smoke test result.** Run: ```bash ./scripts/e2e-smoke-test.sh ``` - Validates the built wheel in an isolated Docker container with a real LLM session. - Requires Docker and `ANTHROPIC_API_KEY`. Do not tag until it passes. - See `context/release-mandate.md` for full details and incident playbook. + Builds a wheel from local source, installs it in an isolated Docker container alongside the real `amplifier` CLI, and runs a real LLM-powered session exercising tool dispatch, agent delegation, and recipe execution. Includes the pristine-import preflight (Step 1b, added in v1.4.1) which imports the wheel into a bare `python:3.12-slim` before any other deps pollute the env, catching undeclared runtime dependencies. Requires Docker and `ANTHROPIC_API_KEY` (or `~/.amplifier/keys.env`). Takes ~5 minutes. Post the output in the PR. CI runs it where environment permits. -4. **Commit, tag, and push:** +5. **No `[tool.uv.sources]` git overrides for `amplifier-core`** in downstream repos. A git-source override on `main` makes the PyPI publish invisible to that repo's users. Verify with: ```bash - git commit -am "chore: bump version to X.Y.Z" - git tag vX.Y.Z - git push origin main --tags + for repo in amplifier amplifier-app-cli amplifier-foundation; do + gh api repos/microsoft/$repo/contents/pyproject.toml --jq '.content' \ + | base64 -d | grep -A2 'amplifier-core.*git' \ + && echo "WARNING: git override in $repo" || echo "$repo: OK" + done ``` -5. **Verify CI triggers.** The `v*` tag triggers `rust-core-wheels.yml`, which builds wheels for all platforms (Linux x86/aarch64, macOS, Windows) and publishes to PyPI. The next PR does not start until PyPI publish is confirmed. +### Who Merges + +**The core owner merges.** Not the PR author, not a delegate, not a reviewer. The core owner verifies all five gates are green, then clicks merge. The merge commit is the release commit. CI tags the commit `vX.Y.Z` and pushes; the `v*` tag triggers `rust-core-wheels.yml` to build wheels for all platforms (Linux x86/aarch64, macOS, Windows) and publish to PyPI. + +This produces a single accountable decision point per release. The intermediate state — "merged but not tagged," "tagged but not published" — is gone. + +### Why This Replaced the Post-Merge Model + +The previous model required a version bump, tag, and push *immediately after merge*. That worked for the original failure mode (`v1.0.7→v1.0.8`, March 2026) but introduced its own: + +- **v1.2.3 (March 2026):** `session_state` crash; a Rust↔Python FFI mismatch all unit tests passed but the wheel was broken. Yanked. +- **v1.2.4 (March 2026):** `_tool_dispatch_context` crash; same class of failure. Yanked. +- **v1.2.4 retag (March 2026):** version files not bumped before tagging; PyPI publish rejected. Re-tagged. +- **PR #63 round-3 review (April 2026):** surfaced the "Python fallback shim until next wheel build" intermediate-state pattern as a structural risk, even when no incident had yet occurred. + +Moving the gates pre-merge means the PR itself is the proof of release readiness. Post-merge mistakes are no longer possible because there is no post-merge work to mishandle. + +### Recovery: When Something Reaches PyPI Broken -### Why the Script Exists +The pre-merge gate is the prevention layer. The Incident Playbook is the recovery layer. Both remain in force. If a broken version reaches PyPI: -The three version files must stay in sync. Manual edits to individual files are error-prone and caused divergence in the past. `scripts/bump_version.py` reads all three, warns if they are already out of sync (canary for prior manual edits), and writes all three atomically. +1. **Yank** the broken version on PyPI immediately (~30 seconds): https://pypi.org/manage/project/amplifier-core/release/X.Y.Z/ → Options → Yank release. +2. **Fix forward** — never reuse a yanked version number. Bump to the next PATCH, run the gates, merge. +3. **Post-mortem** — add the incident to the history table in `context/release-mandate.md`. ---