feat: Phase 1-4 — Layer 4 implementation (engine library + CLI binary)#3
Merged
Conversation
- Phase 1: amplifier_agent_lib foundation (protocol, errors, capabilities, engine, persistence, spawn, protocol_points) - Phase 2: amplifier_agent_cli with Mode A (single-turn argv), admin verbs (doctor, config, cache), TTY detection - Phase 3: CLI Mode B (stdio JSON-RPC), capability negotiation, L14 synthesis, idle timeout - Phase 4: built-in bundle vendoring, XDG cache, post-install hook, cache serialization - 202 tests across lib, CLI, and integration scenarios - All quality gates pass: ruff format/lint, pyright type checking Implementation originally from aaa-source (commits c74316c..4db3100). File manifest: src/amplifier_agent_lib + src/amplifier_agent_cli + tests/ + docs/decisions + config Generated with Amplifier - Phase 1-4 overnight implementation complete Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
The aaa-workspace name was a vestigial placeholder from an earlier phase-1a plan that envisioned a multi-package workspace. The rebuild- from-scratch decision made that structure unnecessary — we ship a single package with two modules (amplifier_agent_lib + amplifier_agent_cli) and one CLI binary (amplifier-agent). This rename makes `pip list` / `uv pip list` show the expected name. No functional change.
Replace the Phase 4 stub handler with the real make_turn_handler factory. Load the bundle once per stdio session via load_and_prepare_cached and reuse it for every turn. Pass bundle_override=prepared to Engine.boot so the bundle is available from the first request.
All 7 cheatsheet §3 verification steps pass:
1. pytest: 208 passed
2. ruff check: clean
3. pyright: 0 errors — fixed CoroutineType/async-with mismatch in
_runtime.py (await create_session first, then async with session)
and updated test mocks to match the corrected API
4. amplifier-agent --version: 0.0.1
5. amplifier-agent doctor: all checks OK
6. amplifier-agent run 'Reply with exactly: pong': structured error
{code: internal} — no TypeError
7. Mode B handshake: agent/initialize → valid response,
turn/submit → structured error (code: tool_execution_failed),
not empty string
These imports are already at module scope (lines 20-21). The local re-imports in _StdioEngine.initialize (lines 211-212) added noise and could mislead readers into thinking the symbols are unavailable at module level. Removed per code review recommendation.
The design checkpoint claims "Built-in bundle, vendored" and "near-instant cold-start once bundle-loading overhead is stripped". Investigation during the 2026-05-19 cheatsheet walk-through showed that only the manifest (~2 KB bundle.md) is vendored — every referenced module (build-up-foundation, loop-streaming, the context module) is git-cloned at first run. The "near-instant" claim only holds on warm cache. This doc captures the question, the current state with evidence, the seven sub-decisions hiding behind "baked in" (D1-D7), and three illustrative archetypes — without recommending any one of them. The answer belongs to a /systems-design pass with adversarial review. The doc is OPEN — it is a design question, not a decision. It does not block the small Thread 1 fix (context-persistent → context-simple in bundle.md) which lands separately.
…i pattern
Cheatsheet §3 ("amplifier-agent run \"...\"") was failing post-TypeError-fix
with a foundation-layer error: context-persistent's transitive dep on
context-simple cannot resolve through foundation's lazy module activator
(activator.py:471 uses `--no-sources`, intentional but it strips
[tool.uv.sources]).
Investigation (across amplifier-app-cli, amplifier-foundation, and the
context-persistent README) revealed three facts:
1. context-persistent is NOT a transcript writer. Its README explicitly:
"No auto-save: Does not persist context back to files". It loads
memory files (AGENTS.md, PROJECT.md) at session start — that's it.
2. amplifier-app-cli does NOT use context-persistent. It defaults to the
foundation bundle, which declares context-simple. Transcript writing
in app-cli is done by CLI-layer hooks (IncrementalSaveHook +
SessionStore), not by any context module.
3. The runtime failure was a direct consequence of (a) misusing
context-persistent + (b) foundation's --no-sources policy.
Fix: bundle.md now mirrors the foundation bundle (context-simple with the
same compaction config). Cheatsheet §3 "Where stuff lives" and §7
"Inspecting state on disk" are corrected; both previously credited
context-persistent for writing $XDG_STATE_HOME/.../context-messages.jsonl,
which is wrong. Note added that session-transcript persistence is a
planned CLI-hook concern, not in this build.
Verification (fresh this session):
- pytest: 208 passed (renamed test_prepared_bundle_declares_context_persistent
→ test_prepared_bundle_declares_context_simple; assertion updated).
- ruff / pyright: clean.
- amplifier-agent run "Reply with one word: pong" → exit 0, no TypeError,
no context-persistent reference, no context-simple install failure.
The pipeline now reaches the orchestrator and returns
{"reply": "Error: No providers available", "turnId": "turn-1"}.
The "No providers available" is a NEW finding: bundle.md's
build-up-foundation include doesn't actually load providers
("No handler for URI: build-up-foundation"). That is part of the broader
baked-in-bundle revisit (docs/designs/2026-05-19-baked-in-bundle-revisit.md,
D1-D7) and is out of scope for this fix.
The `build-up-foundation` bundle is an experimental bundle inside `amplifier-foundation` at the subpath `experiments/build-up/build-up-foundation.md`, NOT at the repo root. Our manifest's include used the bare `git+https://...amplifier-foundation@main` URI, which made foundation's resolver clone the repo and look at the root bundle.md (the standard `foundation` bundle). The name lookup for "build-up-foundation" failed silently with "No handler for URI: build-up-foundation" during prepare. The README at https://github.com/microsoft/amplifier-foundation/blob/main/experiments/build-up/README.md documents the install URI: amplifier bundle add 'git+https://github.com/microsoft/amplifier-foundation@main#subdirectory=experiments/build-up/build-up-foundation.md' --name build-up-foundation The `#subdirectory=experiments/build-up/build-up-foundation.md` fragment tells foundation's resolver where in the repo the bundle entrypoint lives. Verification (fresh this session): - pytest: 208 passed in 11.31s. - ruff / pyright: clean. - amplifier-agent doctor: bundle cache prepares cleanly. - amplifier-agent run "...": no more 'No handler for URI: build-up-foundation' warning in pytest output; the include now resolves. The CLI still returns {"reply": "Error: No providers available", "turnId": "turn-1"} — that is a DIFFERENT, downstream issue: neither our bundle nor build-up-foundation mounts any provider modules, and our `single_turn.py` calls `detect_provider()` only as a credential-presence guard — its return value is discarded. Provider mounting is a separate layer of the baked-in-bundle revisit (see docs/designs/2026-05-19-baked-in-bundle-revisit.md D1 — what gets vendored, D6 — relationship to amplifier-app-cli).
The cheatsheet §3 flagship test (`amplifier-agent run "..."`) was still
failing with {"reply": "Error: No providers available", "turnId": "turn-1"}
after the Mode A wiring fix, the context-persistent → context-simple swap,
and the build-up include URI fix. Investigation surfaced the next layer:
neither our bundle.md, nor build-up-foundation, nor the standard foundation
bundle declares providers. In amplifier, provider mounting is app-layer
policy — both app-cli and openclaw inject providers into
prepared.mount_plan between bundle.prepare() and create_session(). Our
amplifier-agent called detect_provider() as a credential-presence guard
but discarded its return value; no provider module was ever mounted.
This commit implements Option C from the design discussion: detect the
provider from env vars (existing precedence ANTHROPIC → OPENAI → AZURE →
OLLAMA), look up the corresponding module URI in a small catalog, and
inject a mount_plan["providers"] entry after load_and_prepare_cached()
returns. Mirrors openclaw's _inject_user_providers pattern (don't-clobber
semantics; api_key expanded per-invocation so secrets stay out of the
pickle cache).
Changes:
* src/amplifier_agent_cli/provider_sources.py (NEW)
PROVIDER_CATALOG mapping {anthropic, openai, azure-openai, ollama} →
{module, source URI, env_var, default_model}. build_provider_entry()
and inject_provider() helpers.
* src/amplifier_agent_cli/modes/single_turn.py
_TurnSpec gains a `provider` field. run() captures detect_provider()'s
return value. _execute_turn() calls inject_provider() after
load_and_prepare_cached(). Mode B inline _StdioEngine.initialize() does
the same detection + injection (no --provider override in Mode B;
env-var only).
* tests/cli/test_provider_sources.py (NEW, 11 tests)
Catalog shape, env-var expansion, unknown-provider raises, inject
don't-clobber semantics.
* tests/cli/test_mode_a_integration.py
Fixes a pre-existing test-isolation bug — _StubPrepared.mount_plan was
a ClassVar dict, so inject_provider() mutated it across tests. Now an
instance attribute. Adds snapshot capture so tests can assert what was
in mount_plan at create_session() time. Two new tests verify env-var
detection injection and the --provider override path.
Verification (fresh in this session):
pytest 221 passed (210 baseline + 11 provider_sources)
ruff All checks passed!
pyright 0 errors, 0 warnings, 0 informations
ruff format 70 files already formatted
amplifier-agent cache clear → exit 0
amplifier-agent doctor → [INFO] bundle cache: needs prepare
amplifier-agent run "Reply with exactly one word: pong":
exit 0
stdout: {"reply": "pong", "turnId": "turn-1"} ← REAL MODEL REPLY
stderr: no TypeError, no 'No providers available', no context-persistent
This is the first time since the Phase 1-4 implementation landed that the
cheatsheet §3 flagship test reaches the LLM end-to-end.
Out-of-scope remaining issue:
"Include Failed (skipping) ... No handler for URI: build-up-foundation"
still appears in stderr despite the correct #subdirectory= fragment in
our include URI (commit 44db0f4). Foundation's include resolver does not
pick up the experimental build-up bundle name when sourced via git
fragment. The CLI now works DESPITE the include failing because (a) our
bundle.md explicitly declares orchestrator + context, and (b) this
commit's provider injection adds the provider directly to mount_plan.
Whether to keep the include at all, or declare everything explicitly,
is part of docs/designs/2026-05-19-baked-in-bundle-revisit.md (D1, D6).
Merged
6 tasks
manojp99
pushed a commit
that referenced
this pull request
Jun 3, 2026
Adds SpawnAgentParams.runChildProcess?: ChildProcessFactory — a public seam to substitute the subprocess factory used inside SessionHandle. When set, the wrapper invokes the factory in place of child_process.spawn, preserving the same options shape (detached, stdio, env, optional cwd). Useful for: - Sandboxing (e.g. wrapping the child in a container or namespace) - Test doubles (e.g. EventEmitter fakes that drive scripted outputs) - Harness wrapping (e.g. observing the subprocess from outside) ChildProcessFactory is exported as a @public type from index.ts. Closes #3.
manojp99
pushed a commit
that referenced
this pull request
Jun 3, 2026
Wrapper hardening release closing 8 consumer-reported gaps at 0.5.0: #1 configPath surface #2 stderr NDJSON parsing #3 runChildProcess injection #4 display.onEvent dispatch #5 public re-exports #6 Transport dead code (root cause of #2/#4) #7 getEngineInfo() implementation #9 checkProtocolVersion() wired into init path #10 approval API mapped to engine -y/-n + approval.mode Issue #8 in the consumer report was a misread — InitializeParams. mcpConfigPath is intentionally retained in protocol-0.3.0. No type change needed; the schema is canonical and correct. This is a minor bump per 0.x convention even though some changes are BREAKING — the wrapper hasn't shipped a 1.0 yet, so breaking changes ride minor bumps. See CHANGELOG for the BREAKING list. Engine compatibility: requires amplifier-agent >= 0.4.0. Pinned protocol: 0.3.0.
manojp99
added a commit
that referenced
this pull request
Jun 3, 2026
…, approval, getEngineInfo, +5 more) (#36) * feat(wrapper-ts): re-export internal helpers from index.ts (#5) Adds named re-exports from the package entry point so consumers can import internal helpers without reaching into private deep paths: assembleArgv, AssembleArgvInput resolveMcpConfigPath, cleanupSpillFile, McpSpillResult buildEnv, resolveBinaryPath, probeEngineVersion, DEFAULT_ALLOWLIST, BLOCKED_ENV_KEYS, ResolveBinaryPathOptions, BuildEnvOptions Transport, TransportOptions, ExitInfo checkProtocolVersion, VersionCheckResult, VersionCheckOk, VersionCheckFail, CheckProtocolVersionOptions parseRunOutput, STDERR_TAIL_BYTES, SubprocessOutcome makeApprovalHandler, ApprovalAdapter, ApprovalRequest, ApprovalHandler Each export is annotated @public. Closes #5. * feat(wrapper-ts): wire checkProtocolVersion() into init path (#9) spawnAgent() now probes the engine's protocol version once during initialization (via amplifier-agent version --json) and runs checkProtocolVersion() against PROTOCOL_VERSION_REQUIRED_BY_WRAPPER BEFORE constructing a SessionHandle. Mismatch fails fast wrapper-side with AaaError(protocol_version_mismatch), saving a full subprocess roundtrip later. Adds two new SpawnAgentParams fields: - allowProtocolSkew?: boolean — bypass the check (mirrors engine's host_config.allowProtocolSkew) - _engineVersionProbe?: () => Promise<EngineVersionPayload> — test-only injection point for the probe Also bumps PROTOCOL_VERSION_REQUIRED_BY_WRAPPER from "0.2.0" to "0.3.0" to match the engine's current wire protocol (amplifier_agent_lib.protocol.methods.PROTOCOL_VERSION). The wrapper was shipping with a stale pin; the new check would have surfaced this at startup. Closes #9. * feat(wrapper-ts): add runChildProcess injection point (#3) Adds SpawnAgentParams.runChildProcess?: ChildProcessFactory — a public seam to substitute the subprocess factory used inside SessionHandle. When set, the wrapper invokes the factory in place of child_process.spawn, preserving the same options shape (detached, stdio, env, optional cwd). Useful for: - Sandboxing (e.g. wrapping the child in a container or namespace) - Test doubles (e.g. EventEmitter fakes that drive scripted outputs) - Harness wrapping (e.g. observing the subprocess from outside) ChildProcessFactory is exported as a @public type from index.ts. Closes #3. * feat(wrapper-ts)!: wire Transport NDJSON pipeline + dispatch to display.onEvent (#2, #4, #6) The engine emits one JSON object per line on the child subprocess's stderr stream for each wire-protocol notification (progress, result/delta, result/final, thinking/delta, thinking/final, tool/started, tool/completed, approval/request, approval/timeout, plus wire-level error). Before this change the wrapper buffered stderr as raw text and silently dropped every event — the existing Transport class implemented NDJSON parsing but was never wired anywhere (dead code). This change: - Adds parseNdjsonStream(stream, {onJson, onNonJson?}) — a standalone helper extracted from the parsing logic Transport already had. Resolves when the stream emits 'close'. Exported @public. - Wires parseNdjsonStream onto child.stderr inside SessionHandle.makeIterable(). JSON lines are parsed into 'notification' DisplayEvents and dispatched to params.display?.onEvent. Non-JSON lines (and JSON lines, for completeness) are still accumulated into stderrBuf so the stderrTail surface on parseRunOutput remains diagnostically useful. - Extends the DisplayEvent discriminated union with a new {type: 'notification', method: string, params: unknown} variant. **BREAKING**: existing exhaustive switch statements on event.type will no longer be exhaustive without a notification branch. - Threads SpawnAgentParams.display through to SessionHandle so the callback that was previously silently dropped is now actually fired (Issue #4). Closes #2, #4, #6. BREAKING CHANGE: display.onEvent callbacks are now actually invoked with wire-event notifications. Callers that registered onEvent expecting it to be a no-op may observe new event flow. The DisplayEvent union has a new 'notification' variant; exhaustive switch statements need a corresponding branch. * feat(wrapper-ts): surface --config flag via SpawnAgentParams.configPath (#1) Engine PR #27 / v0.4.0 added the --config <path> flag and the host_config layer (approval mode, MCP servers, provider defaults, allowProtocolSkew, etc.). The wrapper had no surface to forward this, so callers had to fall back to AMPLIFIER_AGENT_CONFIG in env.extra. This change: - Adds SpawnAgentParams.configPath?: string (public, @public TSDoc). - Adds AssembleArgvInput.configPath?: string. - assembleArgv emits --config <path> when configPath is set. - Threads configPath through SessionHandleParams to the per-submit argv assembly. Also drive-by adds approvalMode field to AssembleArgvInput (used by #10's commit). The argv-builder now reads input.approvalMode and emits -y / -n / nothing accordingly. Default remains -y for backward compat with callers that haven't opted into the approval API. Closes #1. * feat(wrapper-ts)!: wire approval API to engine -y/-n + approval.mode (#10) Previously, SpawnAgentParams.approval threw AaaError( approval_not_supported_in_v1) whenever set because it required the mid-turn onRequest callback that v1 doesn't support. This change extends SpawnAgentParams.approval to also accept the static-policy shape { mode: 'yes' | 'no' | 'prompt' }, which maps to engine argv: - 'yes' -> -y (auto-allow every tool call) - 'no' -> -n (auto-deny every tool call) - 'prompt' -> emit no flag; engine falls back to host_config.approval.mode or the bundle's TTY-based default. This is how a host hands policy resolution back to the engine. The legacy { onRequest, timeoutMs } form still throws approval_not_supported_in_v1 — the Mode A wire has no mid-turn channel. Mid-turn callbacks will return when WG-4 lands. Engine compatibility: { mode: 'prompt' } requires amplifier-agent >= 0.4.0 (PR #34 added host_config.approval.mode). Closes #10. BREAKING CHANGE: SpawnAgentParams.approval is now a union shape; callers passing { mode } no longer hit approval_not_supported_in_v1. Callers that defensively catch that error need to remove the try/catch when migrating to the mode shape. * feat(wrapper-ts): implement getEngineInfo() — engineVersion + bundleDigest (#7) Closes the Task-9 TODO: getEngineInfo() now returns the values captured during the engine version probe that spawnAgent() runs at init (Issue #9). Previously both fields were hardcoded empty strings. - engineVersion populated from `amplifier-agent version --json` payload's `version` field. - bundleDigest populated from the probe payload's optional `bundleDigest` field. The engine's current `version --json` output (from admin/version_info.py) only emits {version, protocolVersion} — bundleDigest will be empty string until a future engine release exposes it. Forward-compatible: when the engine adds it, the wrapper picks it up automatically with no further changes. DONE_WITH_CONCERNS for the bundleDigest follow-up: filed as an engine-side ask for a future PR. The wrapper does what it can with the data the engine surface exposes today; the contract is wired so the field will populate the moment the engine emits it. Closes #7. * chore(wrapper-ts): rebuild dist after hardening release changes Mirrors PR #29 / #31 pattern: dist/ is tracked so consumers installing from the git tarball get the compiled artifacts without a build step. Regenerated from npm run build after issues #1, #2, #3, #4, #5, #6, #7, #9, #10 landed. * chore(release): bump amplifier-agent-ts to 0.6.0 + CHANGELOG Wrapper hardening release closing 8 consumer-reported gaps at 0.5.0: #1 configPath surface #2 stderr NDJSON parsing #3 runChildProcess injection #4 display.onEvent dispatch #5 public re-exports #6 Transport dead code (root cause of #2/#4) #7 getEngineInfo() implementation #9 checkProtocolVersion() wired into init path #10 approval API mapped to engine -y/-n + approval.mode Issue #8 in the consumer report was a misread — InitializeParams. mcpConfigPath is intentionally retained in protocol-0.3.0. No type change needed; the schema is canonical and correct. This is a minor bump per 0.x convention even though some changes are BREAKING — the wrapper hasn't shipped a 1.0 yet, so breaking changes ride minor bumps. See CHANGELOG for the BREAKING list. Engine compatibility: requires amplifier-agent >= 0.4.0. Pinned protocol: 0.3.0. --------- Co-authored-by: Manoj Prabhakar Paidiparthy <mpaidiparthy@microsoft.com>
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
1 participant
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.
Summary
Implements Phase 1-4 of the AaA v2 design checkpoint — the complete Layer 4 of the 5-layer architecture: the transport-free engine library (
amplifier_agent_lib) and the thin CLI binary (amplifier_agent_cli) that adapts argv and stdio JSON-RPC into engine operations.What ships
Engine library (
src/amplifier_agent_lib/)Engineclass withboot()/dispatch()/submit_turn()/shutdown()lifecycleApprovalSystem,DisplaySystem(per Application Integration Guide §1)[type]prefixbundle.md+ loader + first-invocation prepare-and-cache to$XDG_CACHE_HOME/amplifier-agent/prepared/<version>/CLI binary (
src/amplifier_agent_cli/)amplifier-agent run "prompt"— Mode A (single-turn argv invocation; for shell-out callers)amplifier-agent run --stdio— Mode B (multi-turn JSON-RPC over stdin; for wrapper-driven hosts)amplifier-agent doctor— environment diagnostic (provider keys, XDG paths, bundle cache)amplifier-agent config show— print resolved config with source annotationsamplifier-agent cache clear— invalidate XDG prepared-bundle cacheprompt-when-tty, deny-otherwisewith-y/-noverrides (apt-style)Tests
print()orsys.stdout.writein lib)Quality gates (all clean)
ruff check— 65 files, 0 issuesruff format --check— 65 files, all formattedpyright— 0 errors, 0 warningsArchitecture
agent/shutdownrequest.What's NOT in this PR
Deferred to subsequent phases:
Known follow-ups (non-blocking)
These were flagged during Phase 3 review but are not blocking merge:
stdio_loop.py:331, 375use-32600/-32601where-32603(Internal Error) would be semantically more correctjsonrpc.read_message()andstdio_loopdispatch should addlogging.warning(..., exc_info=True)for production visibilityamplifier_agent_cli/__main__.pystill claims Mode B is stubbed — Phase 3 implemented it; update wordingAddress in a follow-up PR.
Testing this PR locally
Design source
The design checkpoint that drove this implementation lives in the design-staging repo (not yet open-sourced):
aaa-source/docs/designs/aaa-v2-design-checkpoint.md. A copy is referenced fromdocs/decisions/if useful for reviewers.🤖 Generated with Amplifier