Skip to content

feat(protocol): Phase 2.1 — wire spec hardening (generator, schemas, conformance fixtures)#6

Merged
manojp99 merged 12 commits into
mainfrom
feat/phase-2-1-wire-spec-hardening
May 21, 2026
Merged

feat(protocol): Phase 2.1 — wire spec hardening (generator, schemas, conformance fixtures)#6
manojp99 merged 12 commits into
mainfrom
feat/phase-2-1-wire-spec-hardening

Conversation

@manojp99

Copy link
Copy Markdown
Collaborator

Summary

Phase 2.1 of the AaA v2 wrapper-layer + wire-protocol design (docs/designs/2026-05-20-aaa-v2-wrapper-and-wire.md). This phase hardens the wire contract into language-neutral artifacts that the TypeScript and Python wrappers (Phases 2.2 / 2.3) will consume in the next PR.

What this PR ships:

  • src/amplifier_agent_lib/protocol/_gen.py — Python TypedDict → JSON Schema + Markdown generator. CLI invokable via uv run python -m amplifier_agent_lib.protocol._gen <output_dir>. Pure transformation, no side effects.
  • src/amplifier_agent_lib/protocol/spec.md — Generated human-readable normative reference covering protocol version, methods, notifications, error codes, and capability negotiation.
  • src/amplifier_agent_lib/protocol/schemas/*.schema.json — 30 generated JSON Schemas (one per TypedDict) plus error_codes.schema.json for the error catalog.
  • src/amplifier_agent_lib/protocol/conformance/ — New package containing:
    • loader.py — YAML fixture loader + structural validator
    • fixtures/l14_synthesis.yaml — L14 contract both branches (engine emits / engine omits)
    • fixtures/capability_negotiation.yaml — Intersection semantics
    • fixtures/subagent_lineage.yaml — Sub-agent event lineage via parentTurnId
    • fixtures/version_skew.yaml — Strict-refuse handshake (override branch deferred — see follow-up below)
    • fixtures/resume_continuity.yaml — Two-turn --session-id X --resume continuity
  • CI staleness gatetests/test_protocol_gen_staleness.py runs the generator in-process and fails if checked-in artifacts drift from TypedDicts.
  • Wheel packagingpyproject.toml updated via [tool.hatch.build.targets.wheel.force-include] to ship spec.md, schemas/, and fixtures/ in the wheel.
  • Phase 2.1 exit gatetests/test_phase_2_1_exit_gate.py end-to-end roundtrip: regenerate → load → jsonschema-validate test payload → fixture parse → version coherence.

Per-task ceremony tightened vs Phase 2.0c plan: 1 commit per task (12 commits for 11 tasks plus a ruff fixup), task bodies ~50–80 lines instead of ~110, two batch python_check checkpoints instead of per-task.

Test Plan

  • Full test suite: 360 passed in 24.65s (uv run python -m pytest tests/ -q)
  • Phase 2.1 net-new tests: 52/52 pass
  • Ruff: All checks passed!
  • Pyright: 0 errors, 0 warnings, 0 informations
  • Regen smoke: spec.md + 30 schemas roundtrip byte-identical (uv run python -m amplifier_agent_lib.protocol._gen ...)
  • Staleness gate: parametrized per-schema tests pass
  • All 5 conformance fixtures parse + structure-validate

Known issue (NOT a merge blocker)

version_skew.yaml is incomplete. Only the strict-refuse branch is scripted; the override branch (allowProtocolSkew=true) is missing despite the fixture name implying both. Plan 3 (wrapper harnesses) will consume this — should be resolved before that work starts. Either add the override branch to the fixture's script: and assertions: sections, OR rename to version_skew_strict_refuse.yaml and split the override into a separate fixture. Tracked for Plan 3.

Pre-existing failures (NOT caused by this PR)

Four integration tests fail in the local CI env:

  • tests/cli/test_delegation_e2e.py::test_delegation_spawns_child_and_returns_pong
  • tests/cli/test_delegation_e2e.py::test_explorer_bash_tool_mounts_in_child_session
  • tests/cli/test_end_to_end.py::test_phase_2_0c_exit_gate_real_turn_emits_result_events
  • tests/test_resume_continuity.py::test_resume_continuity_two_turns_share_context

These test files were NOT touched by Phase 2.1. They appear to be environmental drift since /verify of PR #5 (where all 311 tests passed) — likely cache state or module-resolution related. Tracked as a separate /debug task.

Out of scope (next PR — Plan 3)

  • TypeScript wrapper amplifier-agent-client-ts (Phase 2.2)
  • Python wrapper amplifier-agent-client-py (Phase 2.3)
  • TS + Py conformance harnesses (Phase 2.5)
  • Cross-language parity lint

Plan 3 co-designs all three above and consumes the artifacts shipped here.

References

Generated with Amplifier

Manoj Prabhakar Paidiparthy added 12 commits May 20, 2026 20:08
- Add _is_typed_dict(), _discover_typed_dicts(), _write_error_codes_schema()
  to protocol/_gen.py
- Update main() to iterate all TypedDicts across methods, notifications,
  and capabilities modules and write one *.schema.json per TypedDict
- Add error_codes.schema.json enumerating all ErrorCode StrEnum values
- Generate schemas/ directory with 30 TypedDict schemas + error_codes.schema.json
- Add schemas/__init__.py marker file
- Add two new tests: test_gen_emits_schema_for_every_typeddict and
  test_gen_error_codes_schema_is_string_enum
@manojp99 manojp99 merged commit a032f97 into main May 21, 2026
1 check passed
manojp99 pushed a commit that referenced this pull request May 26, 2026
Resolves conflicts from PRs #6 (Phase 2.1 wire spec hardening) and #7
(Phase 2.2/2.3/2.5 wrappers + conformance) landing on main while this
branch carried the Mode A pivot work that supersedes Mode B.

Resolution strategy:
- Wrapper source + tests (TypeScript + Python): take OURS. Mode A
  subprocess-driver implementation supersedes Mode B JSON-RPC version
  per the 2026-05-24 Mode A pivot amendment (CR-C breaking change).
- Mode B-only modules brought in by main (display.{py,ts}, jsonrpc.{py,ts},
  l14.{py,ts}, and their tests): DELETED. Mode A pivot eliminated these
  capabilities; deferred to v1.x per amendment §6.
- Conformance fixtures (5 OLD Mode B JSON-RPC fixtures), conformance
  loader, runner_py.py, runner-ts.test.ts, freshness guard tests: take
  OURS. Debug cycle 3 fixed runner_py.py import + added freshness guard.
- Protocol schemas, spec.md, version_info.py, Phase 2.1 tests: take OURS.
  Our branch is on top of main's Phase 2.1 baseline and has additional
  Mode A pivot-related touches.

Regression check: 453 passed, 3 skipped, 0 failed (uv run pytest tests/ -q).
Same green state as pre-merge HEAD (7bd3a80).

Co-Authored-By: Amplifier <amplifier@microsoft.com>
manojp99 pushed a commit that referenced this pull request Jun 3, 2026
…ay.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.
manojp99 pushed a commit that referenced this pull request Jun 3, 2026
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.
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant