Preserve individual hook injection lifetimes - #111
Merged
Merged
Conversation
Generated with Amplifier Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Generated with Amplifier Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Generated with Amplifier Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Collaborator
Author
|
This PR's head The carrier retains the complete change and passed the combined-state native, consumer, and default Docker smoke gates after CLI companion microsoft/amplifier-app-cli#358 landed. See the combined validation and source receipt and the migration notes in #109. No package was published by this review lane. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
This Core-only carrier preserves each hook context injection as an ordered
ContextInjectionitem instead of reducing mixed lifetimes into one scalar result. Each item retains its content, role, ephemeral lifetime, append-to-last-tool-result flag, hook provenance, and event provenance.HookRegistrycontinues to project legacy scalar fields for existing consumers: content is concatenated, the first item supplies the role, and boolean flags are ORed. Existing scalar-only producers remain compatible.The coordinator validates the complete item sequence before context writes, writes durable items once, and returns only ephemeral items as the residual result. Python, gRPC/proto, and Node bindings carry the additive schema and conversions.
Compatibility and generated-stub repairs
The single-handler normalization path preserves all non-injection
HookResultfields, including UI and approval metadata, while updating only the legacy injection projection from the canonical ordered items.The committed Python gRPC stub uses its package-relative sibling import. A canonical, pinned
grpcio-tools==1.78.0generation script produces both committed stubs; Proto Sync runs it and compares complete generated output. The loader remediation and developer instructions point to that command.Test-only follow-up
This final follow-up changes only tests. It asserts the append-only 16-field
HookResultproto schema and exact field numbers, validates the repeatedContextInjectiondescriptor and public alias export, and replaces scheduler-order dependence in request-ID isolation coverage with a deterministic async barrier.Why
The scalar aggregation path collapses mixed durable and ephemeral injections. That loses per-item lifetime and provenance before the coordinator can make the persistence decision. The associated compatibility and generated-stub repairs address the failures discovered on the first draft CI run.
Scope and non-goals
This is the Core carrier, not a claim that the wider CI-journal transition is complete. Dependent context, loop, and CLI work remains local and is not included in this PR.
No Core logging or policy dependency, compaction policy, Core dependency, version change, or production change was added in the test-only follow-up.
Verification
cargo fmt --all -- --checkpassed.pytest tests/ bindings/python/tests/ -m 'not slow'— 1052 passed, 1 skipped, 6 deselected, 2 warnings. The existing workflow excludes the slow WASM integration category.inject_contextretains UI message, suppression, and ordered injection fields.The preceding Core PR head passed its public Rust, Node, and Proto runs after the compatibility repair. This test-only commit requires its own GitHub Actions confirmation; do not infer final status from prior-SHA runs.
Breaking changes
None intended.
context_injectionsis additive, and existing scalar hook-result producers continue to work through the compatibility projection.Draft status
Draft only; do not merge or mark ready. It remains under review until this commit's applicable GitHub Actions checks pass, release prerequisites are reviewed, and downstream compatibility is confirmed.