|
| 1 | +# libvcs Asyncio Support Implementation Plan |
| 2 | + |
| 3 | +## Study Sources |
| 4 | + |
| 5 | +The following reference codebases were studied to inform this design: |
| 6 | + |
| 7 | +| Source | Path | Key Learnings | |
| 8 | +|--------|------|---------------| |
| 9 | +| **CPython asyncio** | `~/study/c/cpython/Lib/asyncio/` | Subprocess patterns, flow control, `communicate()` | |
| 10 | +| **pytest** | `~/study/python/pytest/` | Fixture system internals, parametrization | |
| 11 | +| **pytest-asyncio** | `~/study/python/pytest-asyncio/` | Async fixture wrapping, event loop management | |
| 12 | +| **git** | `~/study/c/git/` | VCS command patterns | |
| 13 | + |
| 14 | +### Key Files Studied |
| 15 | + |
| 16 | +- `cpython/Lib/asyncio/subprocess.py` - High-level async subprocess API |
| 17 | +- `cpython/Lib/asyncio/streams.py` - StreamReader/Writer with backpressure |
| 18 | +- `cpython/Lib/asyncio/base_subprocess.py` - Protocol/Transport implementation |
| 19 | +- `pytest-asyncio/pytest_asyncio/plugin.py` - Fixture wrapping, loop lifecycle |
| 20 | + |
| 21 | +--- |
| 22 | + |
| 23 | +## Decisions |
| 24 | + |
| 25 | +| Decision | Choice | Rationale | |
| 26 | +|----------|--------|-----------| |
| 27 | +| **Scope** | Full stack (subprocess → cmd → sync) | Complete async workflows | |
| 28 | +| **Organization** | Separate `_async/` subpackages | Clean separation, maintainable | |
| 29 | +| **Callbacks** | Async-only for async APIs | Better DX, typing, no runtime overhead | |
| 30 | +| **Testing** | 100% coverage, pytest-asyncio strict mode | Reliability, isolation | |
| 31 | +| **Typing** | Fully typed, no `Any` escapes | Type safety, IDE support | |
| 32 | + |
| 33 | +--- |
| 34 | + |
| 35 | +## DOs |
| 36 | + |
| 37 | +### Subprocess Execution |
| 38 | +- **DO** use `communicate()` for all subprocess I/O (prevents deadlocks) |
| 39 | +- **DO** use `asyncio.timeout()` context manager for timeouts |
| 40 | +- **DO** handle `BrokenPipeError` gracefully (process may exit early) |
| 41 | +- **DO** use try/finally for resource cleanup |
| 42 | + |
| 43 | +### API Design |
| 44 | +- **DO** keep sync and async APIs parallel in `_async/` subpackages |
| 45 | +- **DO** share argument-building logic between sync/async variants |
| 46 | +- **DO** use async-only callbacks for async APIs (no `inspect.isawaitable()`) |
| 47 | +- **DO** provide `wrap_sync_callback()` helper for users with sync callbacks |
| 48 | +- **DO** use `Async` prefix for async classes: `AsyncGit`, `AsyncGitSync` |
| 49 | + |
| 50 | +### Testing |
| 51 | +- **DO** use strict mode for pytest-asyncio |
| 52 | +- **DO** use function-scoped event loops for test isolation |
| 53 | +- **DO** use `@pytest_asyncio.fixture` for async fixtures |
| 54 | +- **DO** use `NamedTuple` with `test_id` for parametrized tests |
| 55 | +- **DO** mirror sync test structure for async tests |
| 56 | + |
| 57 | +### Typing |
| 58 | +- **DO** use `from __future__ import annotations` in all files |
| 59 | +- **DO** use `import typing as t` namespace pattern |
| 60 | +- **DO** provide explicit return type annotations |
| 61 | +- **DO** use Protocol classes for callback types |
| 62 | + |
| 63 | +--- |
| 64 | + |
| 65 | +## DON'Ts |
| 66 | + |
| 67 | +### Subprocess Execution |
| 68 | +- **DON'T** poll `returncode` - use `await proc.wait()` |
| 69 | +- **DON'T** read stdout/stderr manually for bidirectional I/O |
| 70 | +- **DON'T** close event loop in user code |
| 71 | +- **DON'T** mix blocking `subprocess.run()` in async code |
| 72 | +- **DON'T** create new event loops manually |
| 73 | + |
| 74 | +### API Design |
| 75 | +- **DON'T** use union types for callbacks (`None | Awaitable[None]`) |
| 76 | +- **DON'T** break backward compatibility of sync APIs |
| 77 | +- **DON'T** leak event loop details into public APIs |
| 78 | + |
| 79 | +### Testing |
| 80 | +- **DON'T** assume tests run concurrently (they're sequential) |
| 81 | +- **DON'T** close event loop in tests (pytest-asyncio handles cleanup) |
| 82 | +- **DON'T** mismatch fixture scope and loop scope |
| 83 | + |
| 84 | +--- |
| 85 | + |
| 86 | +## Implementation Phases |
| 87 | + |
| 88 | +### Phase 1: Core Async Subprocess (Foundation) |
| 89 | + |
| 90 | +**Goal:** Create async subprocess wrapper matching `SubprocessCommand` API. |
| 91 | + |
| 92 | +**Files to create:** |
| 93 | +- `src/libvcs/_internal/async_subprocess.py` |
| 94 | + |
| 95 | +**Key patterns:** |
| 96 | +```python |
| 97 | +@dataclasses.dataclass |
| 98 | +class AsyncSubprocessCommand: |
| 99 | + args: list[str] |
| 100 | + cwd: pathlib.Path | None = None |
| 101 | + env: dict[str, str] | None = None |
| 102 | + |
| 103 | + async def run(self, *, check: bool = True, timeout: float | None = None) -> tuple[str, str, int]: |
| 104 | + proc = await asyncio.create_subprocess_shell(...) |
| 105 | + async with asyncio.timeout(timeout): |
| 106 | + stdout, stderr = await proc.communicate() |
| 107 | + return stdout.decode(), stderr.decode(), proc.returncode |
| 108 | +``` |
| 109 | + |
| 110 | +**Tests:** |
| 111 | +- `tests/_internal/test_async_subprocess.py` |
| 112 | + |
| 113 | +--- |
| 114 | + |
| 115 | +### Phase 2: Async Run Function |
| 116 | + |
| 117 | +**Goal:** Async equivalent of `_internal/run.py` with output parsing. |
| 118 | + |
| 119 | +**Files to create:** |
| 120 | +- `src/libvcs/_internal/async_run.py` |
| 121 | + |
| 122 | +**Key considerations:** |
| 123 | +- Reuse output parsing logic from `run.py` |
| 124 | +- Async callback protocol: `async def callback(output: str, timestamp: datetime) -> None` |
| 125 | +- Stream output line-by-line using `StreamReader.readline()` |
| 126 | + |
| 127 | +**Tests:** |
| 128 | +- `tests/_internal/test_async_run.py` |
| 129 | + |
| 130 | +--- |
| 131 | + |
| 132 | +### Phase 3: Async Command Classes |
| 133 | + |
| 134 | +**Goal:** Async equivalents of `Git`, `Hg`, `Svn` command classes. |
| 135 | + |
| 136 | +**Files to create:** |
| 137 | +- `src/libvcs/cmd/_async/__init__.py` |
| 138 | +- `src/libvcs/cmd/_async/git.py` - `AsyncGit` |
| 139 | +- `src/libvcs/cmd/_async/hg.py` - `AsyncHg` |
| 140 | +- `src/libvcs/cmd/_async/svn.py` - `AsyncSvn` |
| 141 | + |
| 142 | +**Strategy:** |
| 143 | +- Extract argument-building to shared functions |
| 144 | +- Async methods call `await self.run()` instead of `self.run()` |
| 145 | +- Manager classes (GitRemoteManager, etc.) get async variants |
| 146 | + |
| 147 | +**Tests:** |
| 148 | +- `tests/cmd/_async/test_git.py` |
| 149 | +- `tests/cmd/_async/test_hg.py` |
| 150 | +- `tests/cmd/_async/test_svn.py` |
| 151 | + |
| 152 | +--- |
| 153 | + |
| 154 | +### Phase 4: Async Sync Classes |
| 155 | + |
| 156 | +**Goal:** Async equivalents of `GitSync`, `HgSync`, `SvnSync`. |
| 157 | + |
| 158 | +**Files to create:** |
| 159 | +- `src/libvcs/sync/_async/__init__.py` |
| 160 | +- `src/libvcs/sync/_async/base.py` - `AsyncBaseSync` |
| 161 | +- `src/libvcs/sync/_async/git.py` - `AsyncGitSync` |
| 162 | +- `src/libvcs/sync/_async/hg.py` - `AsyncHgSync` |
| 163 | +- `src/libvcs/sync/_async/svn.py` - `AsyncSvnSync` |
| 164 | + |
| 165 | +**Key patterns:** |
| 166 | +```python |
| 167 | +class AsyncGitSync(AsyncBaseSync): |
| 168 | + async def obtain(self, ...) -> None: |
| 169 | + await self.cmd.clone(...) |
| 170 | + |
| 171 | + async def update_repo(self, ...) -> None: |
| 172 | + await self.cmd.fetch(...) |
| 173 | + await self.cmd.checkout(...) |
| 174 | +``` |
| 175 | + |
| 176 | +**Tests:** |
| 177 | +- `tests/sync/_async/test_git.py` |
| 178 | +- `tests/sync/_async/test_hg.py` |
| 179 | +- `tests/sync/_async/test_svn.py` |
| 180 | + |
| 181 | +--- |
| 182 | + |
| 183 | +### Phase 5: Async pytest Plugin |
| 184 | + |
| 185 | +**Goal:** Async fixture variants for testing. |
| 186 | + |
| 187 | +**Files to modify:** |
| 188 | +- `src/libvcs/pytest_plugin.py` - Add async fixtures |
| 189 | + |
| 190 | +**New fixtures:** |
| 191 | +- `async_create_git_remote_repo` |
| 192 | +- `async_create_hg_remote_repo` |
| 193 | +- `async_create_svn_remote_repo` |
| 194 | +- `async_git_repo`, `async_hg_repo`, `async_svn_repo` |
| 195 | + |
| 196 | +**Pattern:** |
| 197 | +```python |
| 198 | +@pytest_asyncio.fixture(loop_scope="function") |
| 199 | +async def async_git_repo(tmp_path: Path) -> t.AsyncGenerator[AsyncGitSync, None]: |
| 200 | + repo = AsyncGitSync(url="...", path=tmp_path / "repo") |
| 201 | + await repo.obtain() |
| 202 | + yield repo |
| 203 | +``` |
| 204 | + |
| 205 | +--- |
| 206 | + |
| 207 | +## Test Strategy |
| 208 | + |
| 209 | +### pytest Configuration |
| 210 | + |
| 211 | +```toml |
| 212 | +# pyproject.toml |
| 213 | +[tool.pytest.ini_options] |
| 214 | +asyncio_mode = "strict" |
| 215 | +asyncio_default_fixture_loop_scope = "function" |
| 216 | +``` |
| 217 | + |
| 218 | +### Parametrized Test Pattern |
| 219 | + |
| 220 | +```python |
| 221 | +class CloneFixture(t.NamedTuple): |
| 222 | + test_id: str |
| 223 | + clone_kwargs: dict[str, t.Any] |
| 224 | + expected: list[str] |
| 225 | + |
| 226 | +CLONE_FIXTURES = [ |
| 227 | + CloneFixture("basic", {}, [".git"]), |
| 228 | + CloneFixture("shallow", {"depth": 1}, [".git"]), |
| 229 | +] |
| 230 | + |
| 231 | +@pytest.mark.parametrize( |
| 232 | + list(CloneFixture._fields), |
| 233 | + CLONE_FIXTURES, |
| 234 | + ids=[f.test_id for f in CLONE_FIXTURES], |
| 235 | +) |
| 236 | +@pytest.mark.asyncio |
| 237 | +async def test_clone(test_id: str, clone_kwargs: dict, expected: list, ...) -> None: |
| 238 | + ... |
| 239 | +``` |
| 240 | + |
| 241 | +### Coverage Goal |
| 242 | + |
| 243 | +- **Target:** 100% coverage for all async code |
| 244 | +- **Approach:** Mirror sync tests, add async-specific edge cases |
| 245 | +- **Tools:** pytest-cov, pytest-asyncio |
| 246 | + |
| 247 | +--- |
| 248 | + |
| 249 | +## Type Strategy |
| 250 | + |
| 251 | +### Callback Types |
| 252 | + |
| 253 | +```python |
| 254 | +# Sync callback (unchanged) |
| 255 | +ProgressCallback = Callable[[str, datetime], None] |
| 256 | + |
| 257 | +# Async callback (for async APIs only) |
| 258 | +AsyncProgressCallback = Callable[[str, datetime], Awaitable[None]] |
| 259 | + |
| 260 | +# Protocol for type checking |
| 261 | +class AsyncProgressProtocol(t.Protocol): |
| 262 | + async def __call__(self, output: str, timestamp: datetime) -> None: ... |
| 263 | +``` |
| 264 | + |
| 265 | +### Helper for Sync Callback Users |
| 266 | + |
| 267 | +```python |
| 268 | +def wrap_sync_callback( |
| 269 | + sync_cb: Callable[[str, datetime], None] |
| 270 | +) -> AsyncProgressProtocol: |
| 271 | + """Wrap a sync callback for use with async APIs.""" |
| 272 | + async def wrapper(output: str, timestamp: datetime) -> None: |
| 273 | + sync_cb(output, timestamp) |
| 274 | + return wrapper |
| 275 | +``` |
| 276 | + |
| 277 | +--- |
| 278 | + |
| 279 | +## File Structure |
| 280 | + |
| 281 | +``` |
| 282 | +src/libvcs/ |
| 283 | +├── _internal/ |
| 284 | +│ ├── subprocess.py # Existing sync |
| 285 | +│ ├── async_subprocess.py # NEW: Async subprocess |
| 286 | +│ ├── run.py # Existing sync |
| 287 | +│ └── async_run.py # NEW: Async run |
| 288 | +├── cmd/ |
| 289 | +│ ├── git.py # Existing sync |
| 290 | +│ ├── hg.py |
| 291 | +│ ├── svn.py |
| 292 | +│ └── _async/ # NEW |
| 293 | +│ ├── __init__.py |
| 294 | +│ ├── git.py # AsyncGit |
| 295 | +│ ├── hg.py # AsyncHg |
| 296 | +│ └── svn.py # AsyncSvn |
| 297 | +├── sync/ |
| 298 | +│ ├── base.py # Existing sync |
| 299 | +│ ├── git.py |
| 300 | +│ ├── hg.py |
| 301 | +│ ├── svn.py |
| 302 | +│ └── _async/ # NEW |
| 303 | +│ ├── __init__.py |
| 304 | +│ ├── base.py # AsyncBaseSync |
| 305 | +│ ├── git.py # AsyncGitSync |
| 306 | +│ ├── hg.py # AsyncHgSync |
| 307 | +│ └── svn.py # AsyncSvnSync |
| 308 | +└── pytest_plugin.py # Add async fixtures |
| 309 | +``` |
| 310 | + |
| 311 | +--- |
| 312 | + |
| 313 | +## Success Criteria |
| 314 | + |
| 315 | +1. All async APIs pass mypy with strict mode |
| 316 | +2. 100% test coverage for async code |
| 317 | +3. All existing sync tests continue to pass |
| 318 | +4. Documentation updated with async examples |
| 319 | +5. pytest-asyncio strict mode works without warnings |
0 commit comments