ACP workspace policy, backend/session updates, and OAuth connector spec/docs

Made-with: Cursor

Author: Mateusz

.kiro/specs/oauth-connectors-plugin-architecture/design-review.md

@@ -11,7 +11,7 @@ The technical design addresses decoupling of OAuth-oriented backends from core e
 
 | Issue | Resolution in `design.md` |
 |-------|---------------------------|
-| CLI hook timing vs parser build | **Lifecycle ordering** documented: `backend_imports.py` / `discover_plugin_backends` completes synchronously before `ArgumentParserBuilder.build()` in `cli.py`. |
+| CLI hook timing vs parser build | **Lifecycle ordering** documented: `cli.py` import pulls in `backend_imports`, which runs `discover_backends()` (including `discover_plugin_backends()`) **before** `main()` → `parse_cli_args()` → `ArgumentParserBuilder.build()`; alternate imports called out for tests/tools. |
 | Plugin dependency direction for protocols | **`ITokenRefresher` / `ICredentialRotator` / `IOAuthAccountSelector`**: definitions live under `src/core/interfaces/` for core use; **must be re-exported** from `src/core/plugin_api.py` so external packages never deep-import `src.core.interfaces`. |
 | Parsed CLI args not applied to config | **`config_applicator_hook`** on `BackendPluginDefinition`; **`ConfigurationApplicator`** iterates hooks as post-applicator phase via domain-specific applicator pipeline. |
 
@@ -47,11 +47,12 @@ Tracked in `design.md` and `requirements.md` rather than as gate items:
 - **REQ 4.3**: Plugin-owned config schema extension is specified at the design level (hook + documentation); full dynamic Pydantic merge may evolve during implementation.
 - **`Protocol` runtime checks**: Prefer methods over `@property` on `@runtime_checkable` protocols (see `design.md`). `IOAuthAccountSelector` `@runtime_checkable` is optional.
 - **Cross-repo versioning**: `PluginCompatibility` has `core_min_version`/`core_max_version` but runtime version comparison is not yet defined.
+- **Pre-import OAuth filter vs YAML**: Normative section in `requirements.md` plus `design.md` pre-import table and optional `BackendPluginDefinition` registration flags document how `connectors/__init__.py` can honor capability semantics before module import.
 
 ## Final assessment
 
-**Decision**: **GO** (design phase -- subject to normal `spec.json` task approval before implementation).
+**Decision**: **GO** for implementation (`spec.json` marks requirements, design, and tasks **approved**; `ready_for_implementation` is **true**).
 
-**Rationale**: Critical lifecycle, plugin API boundary, ITokenRefresher reconciliation, and configuration application concerns are fully captured in the current design. All P0/P1 audit findings resolved. Remaining items are scope documentation and implementation-detail hardening, not architectural blockers.
+**Rationale**: Critical lifecycle, plugin API boundary, ITokenRefresher reconciliation, configuration application, pre-import capability equivalence, and CLI ordering (default vs alternate paths) are captured. Earlier P0/P1 audit findings remain resolved. Residual items are follow-up scope (deferred modules, runtime semver checks) and normal implementation hardening.
 
-**Next steps**: Approve `tasks.md` in `spec.json` when satisfied; run `/kiro:validate-design` or project checks if the workflow requires a fresh validation stamp after spec edits.
+**Next steps**: Execute `tasks.md` under the project’s Kiro `/kiro:spec-impl` or standard engineering workflow; run `/kiro:validate-design` or repository checks if the team requires a fresh validation stamp after substantive spec edits.

.kiro/specs/oauth-connectors-plugin-architecture/design.md

@@ -45,7 +45,8 @@ The core already has substantial OAuth infrastructure:
 - `streaming_executor.py` defines `ITokenRefresher` protocol (`refresh_token_if_needed(force_reload, session_id, retry_after_seconds) -> bool`) but retains legacy duck-typing helpers: `_is_oauth_auto_refresher` (checks `"oauth-auto" in backend_type` via `getattr`), `_apply_refreshed_auth_header` (accesses `_oauth_credentials` dict via `getattr`), `_get_oauth_auto_selection_strategy` / `_get_oauth_auto_available_account_count` (access `_account_selector` via `getattr`), and `_record_rate_limit` (duck-typed `getattr(token_refresher, "record_rate_limit", None)`).
 - `resilience/scope.py` uses hardcoded `_PERSONAL_BACKEND_TYPES` frozenset with **7 entries** (`antigravity-oauth`, `gemini-cli-cloud-project`, `gemini-oauth-free`, `gemini-oauth-plan`, `openai-codex`, `opencode-zen`, `qwen-oauth`) plus a fallback `"oauth" in normalized or "codex" in normalized` substring heuristic. Also supports runtime config overrides via `resilience.shared_backend_types` / `resilience.personal_backend_types`.
 - `argument_parser_builder.py` contains **9 hardcoded** backend-specific debugging override flags in `_add_debugging_override_arguments` (e.g. `--enable-gemini-oauth-auto-backend-debugging-override`, `--enable-kiro-oauth-auto-backend-debugging-override`). Additionally has 3 in-repo OAuth behavior flags: `--disable-gemini-oauth-fallback`, `--disable-gemini-oauth-reasoning-prompt-injection`, `--allow-oauth-auto-replacement` (deferred — see `requirements.md`).
-- CLI parsing runs before full plugin discovery in the staged initialization path.
+- **CLI vs discovery ordering (default entry point)**: `src/core/cli.py` imports `src.core.services.backend_imports` at module import time; that module calls `discover_backends()` immediately, which runs built-in connector import plus `discover_plugin_backends()` before `main()` calls `parse_cli_args()` → `ArgumentParserBuilder.build()`. A **second** `discover_backends()` during `ApplicationBuilder` startup is idempotent. Therefore, for `python -m src.core.cli`, **full backend/plugin discovery completes before the CLI parser is built**.
+- **Alternate entry points**: Unit tests or tools that construct `ArgumentParserBuilder` (or `ArgumentParser`) **without** first importing `src.core.cli` (or otherwise calling `discover_backends()`) may run with **no** registered plugin CLI hooks. Tests that need hooks must import discovery side effects explicitly or call `discover_backends(force=True)` after test fixtures reset discovery state.
 - Test suite has `pytest.importorskip("llm_proxy_oauth_connectors...")` in `test_qwen_oauth_retry.py` and `test_vendor_prefix.py`.
 
 This spec formalizes and cleans up the remaining implicit contracts.
@@ -60,7 +61,7 @@ This spec formalizes and cleans up the remaining implicit contracts.
   - Extend `BackendCapabilityDescriptor` (`requires_personal_auth`, `is_oauth_based`) to replace name-based classification.
   - Reconcile/extend existing `ITokenRefresher` instead of creating parallel `ICredentialRotator`/`IOAuthAccountSelector` where possible.
   - Add `cli_arguments_hook` and `config_applicator_hook` to `BackendPluginDefinition` (natural extension of existing `post_build_hook`).
-- **CLI lifecycle note (critical)**: Plugin discovery must complete before `ArgumentParserBuilder.build()` or a two-phase parsing strategy is required.
+- **CLI lifecycle note (critical)**: For the supported CLI entry point, plugin discovery already runs at `cli` import time (see above). Implementation work must **preserve** that ordering when adding CLI hooks and must document the **non-default** import path for tests. If a future entry point ever parses before discovery, adopt two-phase parsing or move discovery earlier for that entry point only.
 
 ### Technology Stack
 
@@ -74,7 +75,7 @@ This spec formalizes and cleans up the remaining implicit contracts.
 | Requirement | Summary | Components | Interfaces |
 |-------------|---------|------------|------------|
 | 1.1–1.3 | Core independence (in-scope) | `oauth_detector.py`, `scope.py` | `BackendCapabilityDescriptor` |
-| 2.1–2.3 | Capability declaration | `BackendCapabilityDescriptor` | `BackendCapabilityDescriptor` |
+| 2.1–2.4 | Capability declaration | `BackendCapabilityDescriptor`, `BackendPluginDefinition` | `BackendCapabilityDescriptor`, plugin registration metadata |
 | 3.1–3.5 | Execution decoupling | `streaming_executor.py` | `ITokenRefresher` (relocated + extended), `ICredentialRotator`, `IOAuthAccountSelector` |
 | 4.1–4.5 | Configuration and CLI | `argument_parser_builder.py`, `plugin_api.py`, `ConfigurationApplicator` | `BackendPluginDefinition`, optional schema hook |
 | 5.1–5.4 | Test isolation + packaging exception | Core tests, plugin repo | N/A |
@@ -117,6 +118,18 @@ Add `requires_personal_auth: bool = False` and `is_oauth_based: bool = False` to
 - **Integration**: Update `oauth_detector.py` and `scope.py` to consult these flags (plus any existing neutral signals such as `has_static_credentials` on the class when supplied) instead of static connector lists for in-scope classification.
 - **YAML config update**: All in-repo backends that are OAuth-based or require personal auth must have their `capability_descriptor` sections updated in config YAML templates (e.g., `gemini-oauth-plan`, `gemini-oauth-free`, `openai-codex`, etc.). This ensures the new flags are populated even without plugin hooks.
 
+##### Pre-import connector walk (`src/connectors/__init__.py`)
+
+The auto-discovery loop may call `is_oauth_connector(module_name)` **before** importing a connector module. YAML-backed `BackendCapabilityDescriptor` instances are **not** available at that instant. Align implementation with [Capability signals and discovery timing](requirements.md#capability-signals-and-discovery-timing-normative) in `requirements.md`:
+
+| Approach | When to use | Notes |
+|----------|-------------|-------|
+| **Plugin definition flags** | Entry-point backends | Add `is_oauth_based` / `requires_personal_auth` (names illustrative) to `BackendPluginDefinition` so `oauth_detector` / discovery can read OAuth semantics from registration metadata without hardcoding extracted logical names. |
+| **Core manifest or generated map** | In-repo `pkgutil` modules | Small map keyed by module slug, maintained next to YAML or generated in CI from the same `capability_descriptor` source, read without importing connector implementation. |
+| **Reorder / import-then-filter** | Only if explicitly accepted | Document threat model if OAuth connector module import must run before skip logic. |
+
+`scope.py` operates on configured backends and **can** rely on merged `BackendCapabilityDescriptor` from YAML after config load; the pre-import constraint applies primarily to `oauth_detector` and the `connectors` package walk.
+
 ### Core Plugin API
 
 #### BackendPluginDefinition
@@ -139,6 +152,11 @@ Add `requires_personal_auth: bool = False` and `is_oauth_based: bool = False` to
 @dataclass(frozen=True)
 class BackendPluginDefinition:
     # ... existing fields ...
+    # Registration-time OAuth/capability hints (illustrative names — align with
+    # BackendCapabilityDescriptor in implementation). Required so pre-import
+    # discovery never depends on extracted connector name literals.
+    is_oauth_based: bool = False
+    requires_personal_auth: bool = False
     cli_arguments_hook: Callable[[argparse.ArgumentParser], None] | None = None
     config_applicator_hook: Callable[[argparse.Namespace, AppConfig], AppConfig] | None = None
     # Optional: register_backend_config_models or equivalent — see REQ 4.3 note above.
@@ -176,7 +194,8 @@ class BackendPluginDefinition:
 
 - **`ITokenRefresher`** (existing, relocated): Covers routine and forced token refresh. Already used as the typed parameter in `StreamingExecutor` methods. Remains the primary interface for token refresh.
 - **`ICredentialRotator`** (new, extends `ITokenRefresher`): Adds rate-limit-aware credential rotation, credential snapshot access (replacing `_oauth_credentials` duck-typing), and rate-limit recording (replacing `_record_rate_limit` duck-typing). Implementors MUST also satisfy `ITokenRefresher`.
-- **`IOAuthAccountSelector`** (new, standalone): Covers account selection strategy and available account count. Accessed separately from `ITokenRefresher` via `isinstance` checks on the same object or on a dedicated selector attribute exposed through a public method.
+- **`IOAuthAccountSelector`** (new, standalone): Covers account selection strategy and available account count.
+- **Canonical wiring rule (this spec)**: Multi-account OAuth token refreshers implement **both** `ICredentialRotator` and `IOAuthAccountSelector` on the **same object** passed into `streaming_executor`. Core uses `isinstance(token_refresher, ICredentialRotator)` and `isinstance(token_refresher, IOAuthAccountSelector)` on that object only—**no** `getattr` on `_account_selector`, no nested private selector objects. If a future connector needs a separate selector instance, core may introduce an explicit public accessor method on `ICredentialRotator` in a follow-up spec; it is **out of scope** for the initial migration.
 
 ##### Service interface (illustrative)
 
@@ -228,10 +247,10 @@ class ICredentialRotator(ITokenRefresher, Protocol):
         *,
         retry_after_seconds: float | None = None,
     ) -> None:
-        """Record a rate-limit event for accounting/monitoring.
-        
+        """Record a rate-limit event for accounting/monitoring (synchronous).
+
         Replaces duck-typed getattr(token_refresher, 'record_rate_limit', None).
-        May be sync or async; callers should handle both."""
+        Must be non-blocking and safe to call from async contexts without await."""
         ...
 
 @runtime_checkable
@@ -258,7 +277,7 @@ class IOAuthAccountSelector(Protocol):
 
 - Prefer **methods** (`get_*`) over `@property` on `@runtime_checkable` protocols: CPython's runtime structural checks for `isinstance` are unreliable for protocol properties in many versions.
 - `IOAuthAccountSelector` does **not** require `isinstance` runtime dispatch in the current architecture (it is always accessed via a known code path from `ICredentialRotator` implementors). If this remains the case during implementation, the `@runtime_checkable` decorator is optional and can be omitted to sidestep CPython quirks. Keep it if future polymorphic dispatch is anticipated.
-- If properties are required for backward compatibility during migration, avoid `isinstance` on that protocol; use `getattr` with **public** method names only as a temporary bridge, or use explicit adapter objects owned by core.
+- If properties are required for backward compatibility during migration, avoid `isinstance` on that protocol; use `getattr` with **public** method names only as a temporary bridge, or use explicit adapter objects owned by core. **Do not** bridge via private attributes such as `_account_selector` (requirement 3.4).
 
 ##### Migration path for current duck-typed access
 
@@ -297,7 +316,7 @@ class IOAuthAccountSelector(Protocol):
 **Implementation Notes**
 
 - **Integration**: Remove flags such as `--enable-gemini-oauth-auto-backend-debugging-override`. Add `_add_plugin_arguments` that invokes registered hooks.
-- **Lifecycle ordering (critical)**: Current implementation runs `parse_cli_args()` / `ArgumentParserBuilder.build()` **before** full plugin discovery in staged initialization. This must be resolved (either move discovery earlier or adopt two-phase parsing). Documented as a key risk in `research.md`.
+- **Lifecycle ordering**: Default `cli.py` path already runs `discover_backends()` at import time before `parse_cli_args()`; keep that invariant when wiring hooks. See [Existing Architecture Analysis](#existing-architecture-analysis) and `research.md` for alternate import paths.
 - **Config application**: `ConfigurationApplicator` (or a small helper invoked from it) iterates `config_applicator_hook`s with `Namespace` + `AppConfig` so plugins can inject into `BackendConfig.extra` or other extension fields.
 
 ## Testing Strategy

.kiro/specs/oauth-connectors-plugin-architecture/gap-analysis.md

@@ -16,8 +16,8 @@
 
 ## 2. Requirements Feasibility Analysis
 
-- **Core Independence from Plugin Names**: Feasible. Requires replacing hardcoded lists in `oauth_detector.py` and `scope.py` with **capability flags** on `BackendCapabilityDescriptor` (not a separate global registry). Note: `oauth_detector.py` already has partial dynamic capability via entry-point discovery; the refactor formalizes and completes this.
-- **Capability Declaration**: Feasible. The `BackendCapabilityDescriptor` needs to be extended with `requires_personal_auth` and `is_oauth_based` flags. Pydantic v2 `model_validate` handles new fields with defaults automatically.
+- **Core Independence from Plugin Names**: Feasible. Requires replacing hardcoded lists in `oauth_detector.py` and `scope.py` with **declared capability metadata** (`BackendCapabilityDescriptor` where YAML is loaded; registration-time plugin fields and/or a core manifest for pre-import `connectors/__init__.py` paths — see `requirements.md`). Note: `oauth_detector.py` already has partial dynamic capability via entry-point discovery; the refactor formalizes and completes this.
+- **Capability Declaration**: Feasible. Extend `BackendCapabilityDescriptor` with `requires_personal_auth` and `is_oauth_based`. Mirror OAuth semantics on `BackendPluginDefinition` (or equivalent) for entry points evaluated before connector import. Pydantic v2 `model_validate` handles new descriptor fields with defaults automatically.
 - **Execution Decoupling**: Feasible. `streaming_executor.py` already defines `ITokenRefresher` protocol. New `ICredentialRotator` (extending `ITokenRefresher`) and `IOAuthAccountSelector` protocols must be defined in `src/core/interfaces/`. `ICredentialRotator` also adds `get_current_access_token()` and `record_rate_limit()` to replace remaining duck-typed access patterns. `ITokenRefresher` must be relocated from `streaming_executor.py` to `src/core/interfaces/`.
 - **Configuration and CLI Independence**: Feasible but requires architectural changes. The `ConfigurationApplicator` uses a domain-specific applicator delegation pattern (15+ applicators); plugin hooks should be invoked as a post-applicator phase. The 9 debug override flags and 3 deferred CLI flags need careful classification.
 - **Test Isolation**: Highly feasible. Requires moving OAuth-specific tests (`test_qwen_oauth_retry.py`, relevant cases in `test_vendor_prefix.py`) to the `llm-interactive-proxy-oauth-connectors` repository and using mock plugins for testing discovery in the core repo.
@@ -47,12 +47,12 @@
 
 ## 4. Implementation Complexity & Risk
 
-- **Effort**: **L (1-2 weeks)**. Requires significant refactoring of core execution paths (`streaming_executor.py`), CLI building, and moving a large number of tests across repository boundaries.
+- **Effort**: **L (on the order of one to two weeks)**. Touches core execution (`streaming_executor.py`), discovery/CLI wiring, and a **focused** set of cross-repo test moves (see `tasks.md` Phase 4 — not a bulk test migration).
 - **Risk**: **Medium**. The changes touch critical paths (streaming execution, CLI startup), but the goal is to formalize existing implicit contracts into explicit interfaces, which is a known and safe pattern.
 
 ## 5. Recommendations for Design Phase
 
 - **Preferred Approach**: Option A (Extend Existing Plugin Discovery). Focus on relocating/extending existing `ITokenRefresher`, defining `ICredentialRotator` (extending `ITokenRefresher`) and `IOAuthAccountSelector`, and extending `BackendCapabilityDescriptor`.
 - **Research Needed**:
-  - How to safely allow plugins to register CLI arguments without causing conflicts or breaking the `argparse` lifecycle.
-  - The exact shape of the generic interfaces needed by `streaming_executor.py` to replace `_oauth_credentials`, `_account_selector`, and `record_rate_limit` access. (Largely resolved in `design.md` migration path table.)
+  - Namespace / collision rules for plugin CLI flags (documentation + optional guardrails).
+  - Runtime semver comparison for `PluginCompatibility` if plugins start enforcing `core_max_version` strictly. (Interface shapes for `streaming_executor.py` are resolved in `design.md` migration table.)