feat(sdk)!: upgrade to SDK v0.2.0 with breaking API changes

[Mowri Mohan (mowree)]

BREAKING CHANGE: Requires github-copilot-sdk>=0.2.0,<0.3.0
BREAKING CHANGE: CopilotClient now uses SubprocessConfig
BREAKING CHANGE: session.send() signature changed to send(prompt, attachments=...)

SDK v0.2.0 Upgrade:

• SubprocessConfig replaces raw dict for client initialization
• send(prompt, attachments=...) replaces send({"prompt": ...})
• Vision support: ImageBlock → BlobAttachment passthrough

New Architecture:

• sdk_adapter/ quarantine isolates SDK imports
• config/ YAML files for policy values (Three-Medium)
• contracts/ behavioral specifications

Test Results:

• Windows: 858 passed, 2 skipped (Unix tests), 90.22% coverage
• WSL/Linux: 859 passed, 1 skipped (Windows test), 90.38% coverage

[Copilot]

Pull request overview

Upgrades the provider to github-copilot-sdk v0.2.0 (breaking API changes) and introduces a “Three‑Medium” architecture with SDK isolation, YAML-driven policy, and contract docs.

Changes:

• Replace direct SDK usage with an sdk_adapter/ membrane (quarantined imports, session lifecycle, event/tool extraction).
• Introduce YAML configuration + markdown contracts to define provider behavior (streaming, errors, deny/destroy, SDK protection, observability).
• Restructure tests/fixtures to match updated SDK response shapes and new session creation semantics.

Reviewed changes

Copilot reviewed 69 out of 165 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
tests/sdk_assumptions/test_deny_hook.py	Removes SDK behavioral assumption tests for deny-hook/tool execution.
tests/sdk_assumptions/conftest.py	Removes custom mocked SDK/session harness used by assumption tests.
tests/sdk_assumptions/init.py	Removes package docs for assumption test suite.
tests/sdk_assumptions/README.md	Removes assumption-suite upgrade workflow documentation.
tests/integration/init.py	Removes integration test package marker docstring.
tests/fixtures/sdk_responses.py	Adds realistic SDK response shape fixtures (Data wrapper, etc.).
tests/fixtures/config_capture.py	Adds strict mock client to capture create_session(**kwargs) config for SDK v0.2.0.
tests/fixtures/init.py	Exports new fixture/mocks surface for tests.
tests/init.py	Updates tests package/module docstring.
pyproject.toml	Bumps package to 2.0.0, upgrades SDK dependency to v0.2.x, adds pyright/ruff config, adjusts pytest config.
docs/ARCHITECTURE.md	Adds architecture overview documenting module structure and design decisions.
contracts/streaming-contract.md	Adds streaming behavior specification + anchors.
contracts/sdk-response.md	Adds SDK response extraction contract + anchors.
contracts/sdk-protection.md	Adds defensive invariants for tool capture + session management.
contracts/provider-protocol.md	Adds provider protocol specification + hooks/quality gates anchors.
contracts/observability.md	Adds observability spec and notes partial implementation.
contracts/event-vocabulary.md	Adds SDK→domain event vocabulary and config schema contract.
contracts/error-hierarchy.md	Adds SDK→kernel error translation contract and config schema.
contracts/deny-destroy.md	Adds deny+destroy non-negotiable pattern contract.
contracts/behaviors.md	Adds config-driven policy contract (retry/streaming/model selection, etc.).
amplifier_module_provider_github_copilot/tool_parsing.py	Adds tool call parsing to kernel ToolCall objects.
amplifier_module_provider_github_copilot/tool_capture.py	Removes legacy tool bridge/deny-hook creation module.
amplifier_module_provider_github_copilot/security_redaction.py	Adds secret redaction helpers for log hygiene.
amplifier_module_provider_github_copilot/sdk_adapter/types.py	Adds SDK boundary types + tool/image passthrough helpers.
amplifier_module_provider_github_copilot/sdk_adapter/tool_capture.py	Adds first-turn-only + deduplicated tool capture handler.
amplifier_module_provider_github_copilot/sdk_adapter/extract.py	Adds unified SDK event field extraction helper.
amplifier_module_provider_github_copilot/sdk_adapter/event_helpers.py	Adds SDK event type helpers + tool_request extraction.
amplifier_module_provider_github_copilot/sdk_adapter/client.py	Adds Copilot client wrapper using SubprocessConfig + v0.2 session config/hook model.
amplifier_module_provider_github_copilot/sdk_adapter/_spec_utils.py	Adds SDK-spec discovery via importlib.util.find_spec without importing SDK.
amplifier_module_provider_github_copilot/sdk_adapter/_imports.py	Adds SDK import quarantine module for SDK v0.2 concepts.
amplifier_module_provider_github_copilot/sdk_adapter/init.py	Exposes adapter “membrane” public surface.
amplifier_module_provider_github_copilot/request_adapter.py	Adds ChatRequest→CompletionRequest prompt/attachment adaptation.
amplifier_module_provider_github_copilot/observability.py	Adds YAML-driven observability event emission helpers + lifecycle context.
amplifier_module_provider_github_copilot/model_naming.py	Removes legacy model naming parsing/validation heuristics.
amplifier_module_provider_github_copilot/fake_tool_detection.py	Adds YAML-driven fake tool-call detection + retry logging helpers.
amplifier_module_provider_github_copilot/exceptions.py	Removes custom exception hierarchy in favor of kernel error types.
amplifier_module_provider_github_copilot/converters.py	Removes legacy message/response conversion utilities.
amplifier_module_provider_github_copilot/config/sdk_protection.yaml	Adds SDK protection policy for tool capture + session behavior.
amplifier_module_provider_github_copilot/config/retry.yaml	Adds retry + streaming timing policy values.
amplifier_module_provider_github_copilot/config/observability.yaml	Adds observability policy (event names, status, flags).
amplifier_module_provider_github_copilot/config/models.yaml	Adds provider identity + model catalog/defaults policy.
amplifier_module_provider_github_copilot/config/model_cache.yaml	Adds model cache policy.
amplifier_module_provider_github_copilot/config/fake-tool-detection.yaml	Adds fake tool-call detection policy.
amplifier_module_provider_github_copilot/config/events.yaml	Adds event classification policy mapping SDK event types to domain events.
amplifier_module_provider_github_copilot/config/errors.yaml	Adds error translation policy mappings.
amplifier_module_provider_github_copilot/config/init.py	Adds config package marker for importlib.resources.
amplifier_module_provider_github_copilot/completion.py	Adds new completion lifecycle implementation using v0.2 send/on + tool capture/abort.
amplifier_module_provider_github_copilot/_platform.py	Refactors binary discovery to use membrane spec discovery, caches platform facts.
amplifier_module_provider_github_copilot/_permissions.py	Updates execute-bit repair logic for SDK-bundled binaries (no world-exec).
README.md	Replaces token examples with placeholders.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Samuel Lee (samueljklee)]

Swarm Review: PR #32 — SDK v0.2.0 Upgrade

Reviewed by 3 parallel agents (Anthropic/OpenAI/Gemini) each focused on a different angle: architecture, code correctness, and testing. Findings below are cross-validated against source.

---

Critical — Fix Before Merge

1. extract_response_content() checks .data before .content — silent empty returns
streaming.py:599-601

The function checks hasattr(response, "data") before checking for .content. If an SDK response object has both attributes, .content is never reached. Worse — if .data is None, it recurses into None and returns "" silently, even when valid .content exists on the original object.

# Current (line 599-601):
if hasattr(response, "data"):
    return extract_response_content(response.data, _depth + 1)
# .content check at line 604 is never reached if .data exists

# Fix: check .content first, or guard .data
if hasattr(response, "data") and response.data is not None:
    return extract_response_content(response.data, _depth + 1)

2. CAPIError maps to two conflicting YAML entries — wrong error classification
errors.yaml:58 and errors.yaml:113

CAPIError appears in sdk_patterns for both ContextLengthError (line 58) and InvalidRequestError (line 113). The matching logic at error_translation.py:290-292 returns True on the first sdk_patterns exact name match without requiring string_patterns to also match. So any CAPIError that does not match ContextLengthError string patterns still matches on sdk_patterns alone at line 58, and gets misclassified as ContextLengthError instead of falling through to InvalidRequestError.

Fix: Remove CAPIError from the ContextLengthError mapping (keep it only on InvalidRequestError), or change the matching logic to require both sdk_patterns AND string_patterns when both are specified.

3. Context extraction leaks unredacted secrets
error_translation.py:455-458

Context is extracted from the original (unredacted) message via _extract_context(original_message, ...). If a context extraction regex captures a value containing a credential (e.g., a tool name field with ghp_abc123...), it gets appended to the kernel error unredacted — bypassing the redaction that was applied to safe_message.

# Current (line 456):
context = _extract_context(original_message, mapping.context_extraction)

# Fix: redact extracted context values
context = _extract_context(original_message, mapping.context_extraction)
context = {k: redact_sensitive_text(v) for k, v in context.items()}

4. log_response_text Python default disagrees with YAML secure default
fake_tool_detection.py:36 vs config/fake-tool-detection.yaml:31

Python dataclass default is True, YAML config says false. If YAML config fails to load for any reason, the fallback LoggingConfig() will log full LLM response text (potential PII/secrets).

# Current (line 36):
log_response_text: bool = True

# Fix: match the secure YAML default
log_response_text: bool = False

---

Recommended — Should Fix (follow-up OK)

5. No end-to-end tool capture integration test
The full happy path — complete() → session → SDK tool calls → deny hook → capture → abort → ChatResponse with tool_calls — is never tested as a single integrated flow. Individual pieces pass, but the chain interaction is unvalidated.

6. Membrane violation in models.py:73
Direct import from sdk_adapter.model_translation bypasses the membrane __init__.py. Both symbols are re-exported, so the fix is just changing the import path.

7. client.py imports from outside the adapter (inverted dependency)
sdk_adapter/client.py:20 imports error_translation from the parent package. The membrane should not depend on domain logic — error translation should be injected or called by the consumer.

8. Deleted test suites only partially replaced
tests/sdk_assumptions/ went from 2078 → 226 lines (~11%). tests/integration/ went from 3570 → 504 lines. Multi-model saturation testing and SDK behavioral assumption tests appear largely lost.

9. Missing JWT pattern in security_redaction.py
Covers ghp_, gho_ etc. but misses eyJ... JWT tokens, which are common in SDK error messages.

---

Nice to Have — Non-blocking

• Duplicate cache clears in conftest.py:68-71 (copy-paste, harmless)
• Private symbol re-exports with pyright: ignore in provider.py:84-86 (tech debt)
• Redundant double timeout in provider.py:579,633 (outer + inner use same value)
• is_assistant_message in event_helpers.py:105 uses fragile substring matching instead of exact set
• Architecture debate: "Three-Medium" pattern adds ~2500 lines of YAML configs + contract docs + config loaders for values that could be ~200 lines of typed Python constants. errors.yaml and events.yaml are justified (genuinely tabular); retry.yaml (4 values), model_cache.yaml (1 value), sdk_protection.yaml (safety invariants) are over-engineered for a leaf provider module. Not a merge blocker, but worth discussing before the architecture ossifies.

---

Overall: solid SDK isolation architecture, strong test investment (860 tests, 90%+ coverage), clean provider protocol compliance. The 4 critical items are real bugs with straightforward fixes.

[Mowri Mohan (HDMowri)]

Samuel Lee (@samueljklee) — ALL findings addressed. Thanks for the rigorous swarm review — these were real bugs.

Critical Findings

Finding	Status	Commit	Evidence
C1 .data null guard	✅ Fixed	0514d14	streaming.py:599 — if hasattr(response, "data") and response.data is not None:
C2 CAPIError collision	✅ Fixed	c7222ae	errors.yaml:60 — removed CAPIError from sdk_patterns
C3 Context redaction	✅ Fixed	0514d14	error_translation.py:457 — redact_sensitive_text(v) applied to extracted values
C4 Secure default	✅ Fixed	0514d14	fake_tool_detection.py:37 — log_response_text: bool = False

Recommended Findings

Finding	Status	Commit	Evidence
R5 E2E tests	✅ Added	0514d14	test_provider_coverage.py::TestE2EToolCaptureHappyPath (3 tests)
R6 Membrane import	✅ Fixed	0514d14	models.py:73 — import from .sdk_adapter not internal module
R7 Inverted dependency	✅ Verified	N/A	error_translation.py has zero SDK imports — acceptable
R8 Test coverage	✅ Verified	N/A	980 tests passed, 94% coverage (Windows + WSL)
R9 JWT pattern	✅ Fixed	c7222ae	security_redaction.py:71 — _JWT_PATTERN exists

Note on C1

The suggested .content-first approach would break SDK unwrapping design. Fix: null guard preserves .data → Data.content chain while fixing the silent empty return on None.

Nice to Have — Response

Item	Response
Duplicate cache clears	✅ Accepted — will clean up in follow-up
Private symbol re-exports	✅ Accepted — tech debt ticket filed
Double timeout	✅ Accepted — will consolidate
is_assistant_message substring	✅ Accepted — will use exact set lookup
Three-Medium architecture	📌 Acknowledged — keeping for flexibility (errors.yaml, events.yaml justified; smaller configs borderline but consistent). Will revisit if users never tune these.

Verification: ruff ✅ | pyright ✅ | 980 tests passed

[Samuel Lee (samueljklee)]

Re-reviewed at 0664e2d. All 4 critical findings and 2 recommended findings from the swarm review are verified fixed:

Finding	Status
C1: .data before .content silent empty returns	Fixed (streaming.py:602 — null guard added)
C2: CAPIError double-mapping misclassification	Fixed (errors.yaml:60 — CAPIError removed from ContextLengthError)
C3: Context extraction leaking unredacted secrets	Fixed (error_translation.py:459 — redaction applied to extracted context)
C4: log_response_text default mismatch	Fixed (fake_tool_detection.py:38 — default changed to False)
R5: E2E tool capture test missing	Fixed (test_provider_coverage.py:1021 — 3 test methods)
R6: Membrane violation import	Fixed (models.py:73 — imports via __init__.py)

Bonus: JWT redaction pattern added (security_redaction.py:71).

Remaining open (follow-up OK, not blocking):

• R7: Inverted dependency in sdk_adapter/client.py
• R8: Deleted test suite coverage gaps (~2k net line loss in tests)
• Nice-to-haves: duplicate cache clears in conftest, double timeout, substring matching in is_assistant_message, Three-Medium architecture discussion