fix(proto-gen): fix buf plugin path replacement, GIT_TAG, and generated_dir (#135)

* fix(proto-gen): fix buf plugin path replacement, GIT_TAG, and generated_dir

- Replace silent str.replace() with re.sub() so the protoc-gen-connect-python
  path in buf.gen.yaml is correctly updated regardless of whether it contains
  a relative path, absolute path, or underscore/hyphen naming variant
- Make GIT_TAG overridable via --tag CLI argument; default bumped to service/v0.12.0
- Fix generated_dir to point at src/otdf_python_proto/ (the actual buf output
  location) instead of the non-existent generated/ directory
- Remove unreachable return False after finally block in copy_opentdf_proto_files()
- Implement setup_connect_rpc.py (was empty in source control)

Fixes #133

* test(proto-gen): add unit tests and CI job for generate_connect_proto.py

- 25 unit tests covering all bugs from #133 (fixed) and #134 (open);
  tests for #134 intentionally fail to document the known issue
- Add pytest to otdf-python-proto dev dependencies
- Add proto-unit-tests CI job to test-suite.yaml, running in parallel
  after lint-check with working-directory: otdf-python-proto
- Fix regex from \S+ to \S* so it also matches bare plugin name (no path prefix)

* chore: fix ruff linting issues in test_generate_connect_proto.py

Remove unused imports (call, pytest) and sort import block.

* fix(proto-gen): recurse into nested subdirs when creating __init__.py files

create_init_files() was only walking one level deep, leaving nested packages
like authorization/v2/ and policy/attributes/ without __init__.py files,
causing ImportError on a clean generation run.

Fixes #134

* chore(proto-gen): update uv.lock with pytest dev dependency

* fix(proto-gen): restore default GIT_TAG to service/v0.7.2

* tidy comments

* fix(proto-gen): address Gemini review comments

- Add argparse import and replace fragile manual sys.argv parsing with
  argparse in generate_connect_proto.py
- Use lambda in re.sub replacement to avoid backslash interpretation in
  paths on Windows
- Relax regex to use \s+ and \S* so plugin path prefix is optional
- Replace `uv add` with `uv sync` in setup_connect_rpc.py to avoid
  modifying pyproject.toml/uv.lock for all users

* chore: apply ruff formatting to generate_connect_proto.py

* refactor(proto-gen): replace subprocess rm -rf with shutil.rmtree

Removes shell-out to rm -rf for temp directory cleanup, using the
stdlib shutil.rmtree instead. Updates tests to assert the directory
no longer exists rather than inspecting subprocess calls.

* test(proto-gen): test real argparse parser in TestArgParsing

Replace the private _parse_tag helper (which tested a hand-rolled parser
that no longer exists) with _get_parser(), which replicates the
ArgumentParser setup from main() and calls parse_args() directly.

* fix(build-script): update build_connect_proto.sh for new output directory

- Replace all references to generated/ with src/otdf_python_proto/,
  which is where buf generate now writes files
- Drop the mkdir -p generated/ call (no longer needed)
- Switch uv sync --dev (deprecated flag) to plain uv sync
- Replace silent uv add fallback with a hard error pointing to
  setup_connect_rpc.py, so missing dependencies are caught early

* docs: fix broken link and stale setup instruction

- DEVELOPING.md: replace reference to non-existent PROTOBUF_SETUP.md
  with the correct CONNECT_RPC.md
- otdf-python-proto/README.md: replace `uv add connect-python[compiler]`
  troubleshooting step with `uv run python scripts/setup_connect_rpc.py`,
  which is the correct setup path now that setup_connect_rpc.py exists

* fix: clarify error message and test name from PR review feedback

Improve the connect-python missing error message to accurately state the
dependency is absent (not just unsynced), and rename the test to reflect
what it actually asserts.

Author: b-long

.github/workflows/test-suite.yaml

@@ -32,7 +32,26 @@ jobs:
           uv run ruff check
           uv run ruff format --check
 
-  # Step 2: Build (only after linting passes)
+  # Step 2a: Proto sub-project unit tests (runs in parallel with build)
+  proto-unit-tests:
+    runs-on: ubuntu-22.04
+    needs: lint-check
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Set up uv
+        uses: astral-sh/setup-uv@v6
+        with:
+          enable-cache: true
+          cache-dependency-glob: "otdf-python-proto/uv.lock"
+
+      - name: Run otdf-python-proto unit tests
+        working-directory: otdf-python-proto
+        run: |
+          uv sync --frozen --group dev
+          uv run pytest --tb=short -v tests/
+
+  # Step 2b: Build (only after linting passes)
   build:
     runs-on: ubuntu-22.04
     needs: lint-check
@@ -99,21 +118,22 @@ jobs:
 
   report:
     runs-on: ubuntu-22.04
-    needs: [lint-check, build, unit-tests, integration-tests]
+    needs: [lint-check, proto-unit-tests, build, unit-tests, integration-tests]
     if: always()
     outputs:
       success: ${{ steps.check.outputs.success }}
     steps:
       - name: Check all jobs succeeded
         id: check
         run: |
-          if [[ "${{ needs.lint-check.result }}" == "success" && "${{ needs.build.result }}" == "success" && "${{ needs.unit-tests.result }}" == "success" && "${{ needs.integration-tests.result }}" == "success" ]]; then
+          if [[ "${{ needs.lint-check.result }}" == "success" && "${{ needs.proto-unit-tests.result }}" == "success" && "${{ needs.build.result }}" == "success" && "${{ needs.unit-tests.result }}" == "success" && "${{ needs.integration-tests.result }}" == "success" ]]; then
             echo "success=true" >> $GITHUB_OUTPUT
             echo "✅ All tests passed!"
           else
             echo "success=false" >> $GITHUB_OUTPUT
             echo "❌ Some tests failed:"
             echo "  Lint Check: ${{ needs.lint-check.result }}"
+            echo "  Proto Unit Tests: ${{ needs.proto-unit-tests.result }}"
             echo "  Build: ${{ needs.build.result }}"
             echo "  Unit Tests: ${{ needs.unit-tests.result }}"
             echo "  Integration Tests: ${{ needs.integration-tests.result }}"

docs/DEVELOPING.md

@@ -92,4 +92,4 @@ cd otdf-python-proto
 uv run python scripts/generate_connect_proto.py
 ```
 
-See [`otdf-python-proto/README.md`](../otdf-python-proto/README.md) and [`PROTOBUF_SETUP.md`](./PROTOBUF_SETUP.md) for details.
+See [`otdf-python-proto/README.md`](../otdf-python-proto/README.md) and [`CONNECT_RPC.md`](./CONNECT_RPC.md) for details.

otdf-python-proto/README.md

@@ -170,7 +170,7 @@ If you're migrating from traditional gRPC clients to Connect RPC:
 Install buf: `brew install bufbuild/buf/buf`
 
 ### "protoc-gen-connect_python not found"
-Install with compiler support: `uv add connect-python[compiler]`
+Run the setup script: `uv run python scripts/setup_connect_rpc.py`
 
 ### Import errors after generation
 Ensure `__init__.py` files exist in otdf_python_proto directories

otdf-python-proto/pyproject.toml

@@ -23,8 +23,12 @@ build-backend = "hatchling.build"
 [dependency-groups]
 dev = [
     "mypy-protobuf>=3.6.0",
+    "pytest>=8.0.0",
 ]
 
+[tool.pytest.ini_options]
+testpaths = ["tests"]
+
 [tool.hatch.build.targets.wheel]
 packages = ["src/otdf_python_proto"]
 

otdf-python-proto/scripts/build_connect_proto.sh

@@ -44,28 +44,28 @@ fi
 
 echo "✓ uv is available"
 
-# Install dependencies if needed
+# Install dependencies
 echo "Installing/updating dependencies..."
 cd "$PROTO_GEN_DIR"
-uv sync --dev
+uv sync
 
 # Check if connect-python is available
 if ! uv run python -c "import connectrpc" 2>/dev/null; then
-    echo "Installing connect-python[compiler]..."
-    uv add "connect-python[compiler]>=0.4.2"
+    echo "Error: connect-python is not in the installed dependencies."
+    echo "It may need to be added first. Run: uv run python scripts/setup_connect_rpc.py"
+    echo "Then re-run this script."
+    exit 1
 fi
 
 echo "✓ connect-python is available"
 
 # Clean up previous generated files
+OUTPUT_DIR="$PROTO_GEN_DIR/src/otdf_python_proto"
 echo "Cleaning up previous generated files..."
-if [[ -d "generated" ]]; then
-    rm -rf generated/*
+if [[ -d "$OUTPUT_DIR" ]]; then
+    rm -rf "${OUTPUT_DIR:?}"/*
 fi
 
-# Create generated directory
-mkdir -p generated
-
 # Run the generation
 echo "Generating Connect RPC protobuf files..."
 uv run python scripts/generate_connect_proto.py "$@"
@@ -75,16 +75,12 @@ if [[ $? -eq 0 ]]; then
     echo "✓ Connect RPC generation complete!"
     echo ""
     echo "Generated files:"
-    echo "  - generated/*_pb2.py      (Protobuf message classes)"
-    echo "  - generated/*_pb2.pyi     (Type stubs)"
-    echo "  - generated/*_connect.py  (Connect RPC clients)"
+    echo "  - src/otdf_python_proto/**/*_pb2.py      (Protobuf message classes)"
+    echo "  - src/otdf_python_proto/**/*_pb2.pyi     (Type stubs)"
+    echo "  - src/otdf_python_proto/**/*_connect.py  (Connect RPC clients)"
     echo ""
     echo "Legacy gRPC files (if generated):"
-    echo "  - generated/legacy_grpc/*_pb2_grpc.py (gRPC stubs)"
-    echo ""
-    echo "Usage examples:"
-    echo "  cd .."
-    echo "  python examples/connect_rpc_client_example.py"
+    echo "  - src/otdf_python_proto/legacy_grpc/**/*_pb2_grpc.py (gRPC stubs)"
     echo ""
     echo "For more information, see:"
     echo "  - docs/CONNECT_RPC.md"

otdf-python-proto/scripts/generate_connect_proto.py

@@ -8,6 +8,9 @@
 4. Optionally generates legacy gRPC clients for backward compatibility
 """
 
+import argparse
+import re
+import shutil
 import subprocess
 import sys
 from pathlib import Path
@@ -43,9 +46,9 @@ def check_dependencies() -> bool:
     return True
 
 
-def copy_opentdf_proto_files(proto_gen_dir: Path) -> bool:
+def copy_opentdf_proto_files(proto_gen_dir: Path, git_tag: str | None = None) -> bool:
     """Clone OpenTDF platform repository and copy all proto files."""
-    GIT_TAG = "service/v0.7.2"
+    GIT_TAG = git_tag or "service/v0.7.2"
     REPO_URL = "https://github.com/opentdf/platform.git"
 
     temp_repo_dir = proto_gen_dir / "temp_platform_repo"
@@ -57,7 +60,7 @@ def copy_opentdf_proto_files(proto_gen_dir: Path) -> bool:
     try:
         # Remove existing temp directory if it exists
         if temp_repo_dir.exists():
-            subprocess.run(["rm", "-rf", str(temp_repo_dir)], check=True)
+            shutil.rmtree(temp_repo_dir)
 
         print(f"Cloning OpenTDF platform repository (tag: {GIT_TAG})...")
 
@@ -120,17 +123,15 @@ def copy_opentdf_proto_files(proto_gen_dir: Path) -> bool:
     finally:
         # Clean up temp directory
         if temp_repo_dir.exists():
-            subprocess.run(["rm", "-rf", str(temp_repo_dir)], check=False)
+            shutil.rmtree(temp_repo_dir)
 
-    return False
 
-
-def download_proto_files(proto_gen_dir: Path) -> bool:
+def download_proto_files(proto_gen_dir: Path, git_tag: str | None = None) -> bool:
     """Download proto files from OpenTDF platform."""
     print("Copying proto files from OpenTDF platform...")
 
     try:
-        return copy_opentdf_proto_files(proto_gen_dir)
+        return copy_opentdf_proto_files(proto_gen_dir, git_tag=git_tag)
     except Exception as e:
         print(f"Error getting proto files: {e}")
         return False
@@ -152,14 +153,15 @@ def run_buf_generate(proto_gen_dir: Path) -> bool:
         connect_plugin_path = result.stdout.strip()
         print(f"Using Connect plugin at: {connect_plugin_path}")
 
-        # Update buf.gen.yaml with the correct path
+        # Update buf.gen.yaml with the correct absolute path for the local plugin
         buf_gen_path = proto_gen_dir / "buf.gen.yaml"
         with buf_gen_path.open() as f:
             content = f.read()
 
-        # Replace the local plugin path
-        updated_content = content.replace(
-            "- local: protoc-gen-connect-python", f"- local: {connect_plugin_path}"
+        updated_content = re.sub(
+            r"- local:\s+\S*protoc-gen-connect[_-]python\S*",
+            lambda _: f"- local: {connect_plugin_path}",
+            content,
         )
 
         with buf_gen_path.open("w") as f:
@@ -189,13 +191,9 @@ def run_buf_generate(proto_gen_dir: Path) -> bool:
 
 def create_init_files(generated_dir: Path) -> None:
     """Create __init__.py files in generated directories."""
-    # Create __init__.py in main generated directory
-    (generated_dir / "__init__.py").touch()
-
-    # Create __init__.py files in any subdirectories
-    for subdir in generated_dir.iterdir():
-        if subdir.is_dir():
-            (subdir / "__init__.py").touch()
+    for dirpath in [generated_dir, *generated_dir.rglob("*")]:
+        if dirpath.is_dir():
+            (dirpath / "__init__.py").touch()
 
 
 def _fix_ignore_if_default_value(proto_files_dir):
@@ -245,7 +243,7 @@ def main():
     # Get the proto-gen directory (parent of scripts)
     proto_gen_dir = Path(__file__).parent.parent
     proto_files_dir = proto_gen_dir / "proto-files"
-    generated_dir = proto_gen_dir / "generated"
+    generated_dir = proto_gen_dir / "src" / "otdf_python_proto"
 
     # Check dependencies
     if not check_dependencies():
@@ -255,10 +253,19 @@ def main():
     proto_files_dir.mkdir(exist_ok=True)
     generated_dir.mkdir(exist_ok=True)
 
+    # Parse arguments
+    parser = argparse.ArgumentParser(description="OpenTDF Connect RPC Client Generator")
+    parser.add_argument("--tag", help="Git tag to use for OpenTDF platform")
+    parser.add_argument(
+        "--download", action="store_true", help="Force download of proto files"
+    )
+    args = parser.parse_args()
+    git_tag = args.tag
+
     # Download proto files (optional - can use existing files)
     if (
-        "--download" in sys.argv or not any(proto_files_dir.glob("**/*.proto"))
-    ) and not download_proto_files(proto_gen_dir):
+        args.download or not any(proto_files_dir.glob("**/*.proto"))
+    ) and not download_proto_files(proto_gen_dir, git_tag=git_tag):
         return 1
 
     # Check if we have any proto files

otdf-python-proto/scripts/setup_connect_rpc.py

@@ -0,0 +1,26 @@
+#!/usr/bin/env python3
+"""Setup script for Connect RPC dependencies.
+
+Run this script once to install the required tools before generating proto files:
+
+    uv run python scripts/setup_connect_rpc.py
+"""
+
+import subprocess
+import sys
+
+
+def main():
+    """Install Connect RPC compiler dependencies."""
+    print("Installing Connect RPC dependencies...")
+    subprocess.run(
+        ["uv", "sync"],
+        check=True,
+    )
+    print("✓ Connect RPC dependencies installed.")
+    print("  Run 'uv run python scripts/generate_connect_proto.py' to generate files.")
+    return 0
+
+
+if __name__ == "__main__":
+    sys.exit(main())