fix(ci): harden the lint gate and enable Ruff security rules (#537, #539)

CONTRIBUTING.md

@@ -49,6 +49,8 @@ bash ./scripts/doctor.sh
 
 `doctor.sh` is the primary repository validation entrypoint. It currently runs locked-environment sync, dependency compatibility checks, `pre-commit`, `mypy`, `pytest`, the repository coverage gate, package build, and a built-wheel smoke test.
 
+Ruff runs with the `S` (flake8-bandit) security rules enabled for shipped code. `tests/**` is exempt from `S` because test modules intentionally use asserts and fake credentials; do not widen that exemption beyond `tests/**`.
+
 If you change shell scripts, also run `bash -n` on each modified script, for example:
 
 ```bash

pyproject.toml

@@ -72,7 +72,12 @@ line-length = 100
 src = ["src"]
 
 [tool.ruff.lint]
-select = ["E", "F", "I", "B", "UP"]
+select = ["E", "F", "I", "B", "UP", "S"]
+
+[tool.ruff.lint.per-file-ignores]
+# Bandit-style security rules target shipped code. Test modules intentionally use
+# asserts and hardcoded fake credentials, mirroring codex-a2a's configuration.
+"tests/**" = ["S"]
 
 [tool.ruff.format]
 quote-style = "double"

scripts/README.md

@@ -17,13 +17,14 @@ Executable scripts live in this directory. This file is the entry index for the
 - [`dependency_health.sh`](./dependency_health.sh): development dependency review entrypoint (`sync`/`pip check` + outdated + dev audit), while blocking CI/publish audits focus on runtime dependencies
 - [`check_coverage.py`](./check_coverage.py): enforces the overall coverage floor and per-file minimums for critical modules
 - [`find_thin_wrappers.py`](./find_thin_wrappers.py): static analysis helper that reports local functions with low caller counts and flags likely thin forwarding wrappers for manual abstraction review
-- [`lint.sh`](./lint.sh): lint helper
+- [`lint.sh`](./lint.sh): lint entrypoint used by CI; runs the repository `pre-commit` pipeline and **rejects hook-generated rewrites** instead of retrying them, so an unformatted or un-fixed commit cannot pass the gate
 - [`smoke_test_built_cli.sh`](./smoke_test_built_cli.sh): built-artifact smoke test for the released CLI runtime; defaults to the only local wheel, supports explicit wheel/sdist paths, and rejects ambiguous local artifact selection
 
 ## Notes
 
 - `doctor.sh` and `dependency_health.sh` intentionally remain separate entrypoints and share common prerequisites through [`health_common.sh`](./health_common.sh).
 - `doctor.sh` covers the default local validation baseline, while `dependency_health.sh` remains focused on standalone dependency review and audit flow.
 - `doctor.sh` stops early when `pre-commit` rewrites files so you can review the changes and rerun `doctor.sh` from the updated worktree.
+- `lint.sh` shares the same working-tree fingerprint check through [`health_common.sh`](./health_common.sh): auto-fixing hooks (`trailing-whitespace`, `end-of-file-fixer`, `ruff --fix`, `ruff format`) make the gate fail instead of being masked by a retry.
 - [`.github/dependabot.yml`](../.github/dependabot.yml) prefers a single weekly grouped Dependabot PR for `uv`, while `dependency_health.sh` remains the explicit review/audit entrypoint.
 - Repository-owned compatibility probes remain separate from the default regression path. See [`../docs/conformance.md`](../docs/conformance.md).

scripts/conformance_probe.py

@@ -3,6 +3,7 @@
 import argparse
 import json
 import os
+import shutil
 import subprocess
 from collections.abc import Callable
 from pathlib import Path
@@ -177,11 +178,18 @@ def main() -> int:
     args.output_dir.mkdir(parents=True, exist_ok=True)
     results, card = run_probes(args.base_url.rstrip("/"), token)
     failures = [result for result in results if result["status"] == "failed"]
+    git_executable = shutil.which("git")
+    if git_executable is None:
+        parser.error("git executable is required to record the repository commit")
     report = {
         "schema_version": 1,
         "scope": "repository-owned-a2a-1.0-compatibility-probes",
         "sut_url": args.base_url,
-        "repo_commit": subprocess.check_output(["git", "rev-parse", "HEAD"], text=True).strip(),
+        # The git executable is resolved via shutil.which and invoked with a
+        # fixed argument list, so this is not untrusted input execution.
+        "repo_commit": subprocess.check_output(  # noqa: S603
+            [git_executable, "rev-parse", "HEAD"], text=True
+        ).strip(),
         "summary": {
             "passed": len(results) - len(failures),
             "failed": len(failures),

scripts/doctor.sh

@@ -4,33 +4,21 @@ set -euo pipefail
 # shellcheck source=./health_common.sh
 source "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/health_common.sh"
 
-doctor_repo_state_fingerprint() {
-  {
-    git diff --no-ext-diff --binary --cached -- .
-    git diff --no-ext-diff --binary -- .
-    while IFS= read -r -d '' path; do
-      printf 'untracked %s\n' "$path"
-      cat "$path"
-      printf '\n'
-    done < <(git ls-files --others --exclude-standard -z)
-  } | git hash-object --stdin
-}
-
 run_doctor_fix_phase() {
   local pre_commit_exit_code=0
   local repo_state_before
   local repo_state_after
 
   echo "[doctor] run fix phase"
-  repo_state_before="$(doctor_repo_state_fingerprint)"
+  repo_state_before="$(repo_state_fingerprint)"
 
   if uv run pre-commit run --all-files; then
     pre_commit_exit_code=0
   else
     pre_commit_exit_code=$?
   fi
 
-  repo_state_after="$(doctor_repo_state_fingerprint)"
+  repo_state_after="$(repo_state_fingerprint)"
   if [[ "$repo_state_before" != "$repo_state_after" ]]; then
     echo "[doctor] pre-commit modified files; review the changes and rerun doctor" >&2
     exit 1

scripts/health_common.sh

@@ -21,3 +21,17 @@ run_shared_repo_health_prerequisites() {
   echo "[${label}] verify dependency compatibility"
   uv pip check
 }
+
+# Fingerprint of tracked and untracked working-tree state. Used to detect
+# whether a tool rewrote repository files instead of only reporting on them.
+repo_state_fingerprint() {
+  {
+    git diff --no-ext-diff --binary --cached -- .
+    git diff --no-ext-diff --binary -- .
+    while IFS= read -r -d '' path; do
+      printf 'untracked %s\n' "$path"
+      cat "$path"
+      printf '\n'
+    done < <(git ls-files --others --exclude-standard -z)
+  } | git hash-object --stdin
+}

scripts/lint.sh

@@ -2,18 +2,42 @@
 set -euo pipefail
 
 # Run the repo canonical lint pipeline to keep local and CI checks identical.
+SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+# shellcheck source=./health_common.sh
+source "${SCRIPT_DIR}/health_common.sh"
+
+cd "$ROOT_DIR"
+
 max_retries="${PRE_COMMIT_MAX_RETRIES:-3}"
 retry_delay_seconds="${PRE_COMMIT_RETRY_DELAY_SECONDS:-5}"
 
 attempt=1
 while true; do
+  state_before="$(repo_state_fingerprint)"
+
+  pre_commit_status=0
   if uv run pre-commit run --all-files; then
+    pre_commit_status=0
+  else
+    pre_commit_status=$?
+  fi
+
+  # Auto-fixing hooks (trailing-whitespace, end-of-file-fixer, ruff --fix,
+  # ruff-format) exit non-zero when they rewrite files, so a retry would turn a
+  # real failure into a green run. Report it as a failure instead of retrying.
+  if [[ "$(repo_state_fingerprint)" != "${state_before}" ]]; then
+    echo "ERROR: pre-commit rewrote repository files; the lint gate rejects hook-generated edits." >&2
+    echo "Review and commit the changes, then rerun." >&2
+    exit 1
+  fi
+
+  if (( pre_commit_status == 0 )); then
     break
   fi
 
   if (( attempt >= max_retries )); then
     echo "ERROR: pre-commit failed after ${attempt} attempts." >&2
-    exit 1
+    exit "${pre_commit_status}"
   fi
 
   echo "WARN: pre-commit failed on attempt ${attempt}/${max_retries}, retrying in ${retry_delay_seconds}s..." >&2

src/opencode_a2a/jsonrpc/handlers/provider_discovery.py

@@ -7,6 +7,7 @@
 from starlette.responses import Response
 
 from ...contracts.extensions import PROVIDER_DISCOVERY_ERROR_BUSINESS_CODES
+from ...opencode_upstream_client import UpstreamContractError
 from ..dispatch import ExtensionHandlerContext
 from ..error_responses import invalid_params_error
 from ..methods import (
@@ -93,7 +94,8 @@ async def handle_provider_discovery_request(
     )
     if upstream_error is not None:
         return upstream_error
-    assert raw_result is not None
+    if raw_result is None:
+        raise UpstreamContractError("OpenCode provider discovery returned no payload")
 
     try:
         raw_providers, default_by_provider, connected = _extract_provider_catalog(raw_result)

src/opencode_a2a/jsonrpc/handlers/session_lifecycle.py

@@ -1,7 +1,7 @@
 from __future__ import annotations
 
 import logging
-from typing import Any
+from typing import Any, cast
 
 from starlette.requests import Request
 from starlette.responses import Response
@@ -355,7 +355,7 @@ async def handle_session_lifecycle_request(
                 )
                 result = {"items": _normalize_session_status_items(raw_result)}
             elif method == context.method_get_session:
-                assert session_id is not None
+                session_id = cast(str, session_id)
                 raw_result = await context.upstream_client.get_session(
                     session_id,
                     **routing_kwargs,
@@ -367,7 +367,7 @@ async def handle_session_lifecycle_request(
                     )
                 result = {"item": item}
             elif method == context.method_get_session_children:
-                assert session_id is not None
+                session_id = cast(str, session_id)
                 raw_result = await context.upstream_client.list_child_sessions(
                     session_id,
                     **routing_kwargs,
@@ -381,14 +381,14 @@ async def handle_session_lifecycle_request(
                     ]
                 }
             elif method == context.method_get_session_todo:
-                assert session_id is not None
+                session_id = cast(str, session_id)
                 raw_result = await context.upstream_client.get_session_todo(
                     session_id,
                     **routing_kwargs,
                 )
                 result = {"items": _normalize_todo_items(raw_result)}
             elif method == context.method_get_session_diff:
-                assert session_id is not None
+                session_id = cast(str, session_id)
                 query = {"messageID": message_id} if message_id else None
                 raw_result = await context.upstream_client.get_session_diff(
                     session_id,
@@ -397,8 +397,8 @@ async def handle_session_lifecycle_request(
                 )
                 result = {"items": _normalize_diff_items(raw_result)}
             elif method == context.method_get_session_message:
-                assert session_id is not None
-                assert message_id is not None
+                session_id = cast(str, session_id)
+                message_id = cast(str, message_id)
                 raw_result = await context.upstream_client.get_message(
                     session_id,
                     message_id,
@@ -412,7 +412,7 @@ async def handle_session_lifecycle_request(
                     )
                 result = {"item": item}
             elif method == context.method_fork_session:
-                assert session_id is not None
+                session_id = cast(str, session_id)
                 raw_result = await context.upstream_client.fork_session(
                     session_id,
                     request=fork_request,
@@ -422,14 +422,14 @@ async def handle_session_lifecycle_request(
                 forked_session_id = item["id"]
                 result = {"item": item}
             elif method == context.method_share_session:
-                assert session_id is not None
+                session_id = cast(str, session_id)
                 raw_result = await context.upstream_client.share_session(
                     session_id,
                     **routing_kwargs,
                 )
                 result = {"item": _normalize_session_summary(raw_result)}
             elif method == context.method_summarize_session:
-                assert session_id is not None
+                session_id = cast(str, session_id)
                 raw_result = await context.upstream_client.summarize_session(
                     session_id,
                     request=summarize_request,
@@ -439,28 +439,29 @@ async def handle_session_lifecycle_request(
                     raise ValueError("Upstream summarize response must be a boolean")
                 result = {"ok": raw_result, "session_id": session_id}
             elif method == context.method_revert_session:
-                assert session_id is not None
+                session_id = cast(str, session_id)
                 raw_result = await context.upstream_client.revert_session(
                     session_id,
                     request=revert_request,
                     **routing_kwargs,
                 )
                 result = {"item": _normalize_session_summary(raw_result)}
             elif method == context.method_unrevert_session:
-                assert session_id is not None
+                session_id = cast(str, session_id)
                 raw_result = await context.upstream_client.unrevert_session(
                     session_id,
                     **routing_kwargs,
                 )
                 result = {"item": _normalize_session_summary(raw_result)}
-            else:
-                assert method == context.method_unshare_session
-                assert session_id is not None
+            elif method == context.method_unshare_session:
+                session_id = cast(str, session_id)
                 raw_result = await context.upstream_client.unshare_session(
                     session_id,
                     **routing_kwargs,
                 )
                 result = {"item": _normalize_session_summary(raw_result)}
+            else:
+                raise UpstreamContractError(f"Unsupported session lifecycle method: {method}")
 
             await session_claim.finalize()
             if forked_session_id is not None and identity:

src/opencode_a2a/jsonrpc/handlers/session_queries.py

@@ -3,12 +3,13 @@
 import logging
 from collections.abc import Callable
 from functools import partial
-from typing import Any
+from typing import Any, cast
 
 from starlette.requests import Request
 from starlette.responses import Response
 
 from ...contracts.extensions import SESSION_QUERY_ERROR_BUSINESS_CODES
+from ...opencode_upstream_client import UpstreamContractError
 from ..dispatch import ExtensionHandlerContext
 from ..error_responses import invalid_params_error, session_not_found_error
 from ..methods import (
@@ -97,18 +98,17 @@ async def _invoke_session_query() -> Any:
                 params=query,
                 **routing_kwargs,
             )
-        assert session_id is not None
         list_messages_kwargs: dict[str, Any] = {"params": query}
         if workspace_id is not None:
             list_messages_kwargs["workspace_id"] = workspace_id
         return await context.upstream_client.list_messages(
-            session_id,
+            cast(str, session_id),
             **list_messages_kwargs,
         )
 
     on_not_found: Callable[[], Response] | None = None
     if base_request.method == context.method_get_session_messages:
-        assert session_id is not None
+        session_id = cast(str, session_id)
         on_not_found = partial(
             context.error_response,
             base_request.id,
@@ -126,7 +126,8 @@ async def _invoke_session_query() -> Any:
     )
     if upstream_error is not None:
         return upstream_error
-    assert raw_result is not None
+    if raw_result is None:
+        raise UpstreamContractError("OpenCode session query returned no payload")
 
     try:
         if base_request.method == context.method_list_sessions:
@@ -150,7 +151,7 @@ async def _invoke_session_query() -> Any:
                 mapped.append(task)
         items: list[dict[str, Any]] = _apply_session_query_limit(mapped, limit=limit)
     else:
-        assert session_id is not None
+        session_id = cast(str, session_id)
         mapped = []
         for item in raw_items:
             message = _as_a2a_message(session_id, item)

src/opencode_a2a/jsonrpc/handlers/workspace_control.py

@@ -1,7 +1,7 @@
 from __future__ import annotations
 
 import logging
-from typing import Any
+from typing import Any, cast
 
 from starlette.requests import Request
 from starlette.responses import Response
@@ -11,6 +11,7 @@
     request_has_capability,
 )
 from ...contracts.extensions import WORKSPACE_CONTROL_ERROR_BUSINESS_CODES
+from ...opencode_upstream_client import UpstreamContractError
 from ..dispatch import ExtensionHandlerContext
 from ..error_responses import invalid_params_error
 from ..models import JSONRPCRequest
@@ -316,7 +317,7 @@ async def handle_workspace_control_request(
         )
         if request_error is not None:
             return request_error
-        assert request_body is not None
+        request_body = cast(dict[str, Any], request_body)
         request_validation_error = _validate_workspace_request(
             context,
             base_request.id,
@@ -336,8 +337,7 @@ async def _invoke_workspace_method() -> Any:
         if method_key == "create_workspace":
             return await context.upstream_client.create_workspace(request_body or {})
         if method_key == "remove_workspace":
-            assert workspace_id is not None
-            return await context.upstream_client.remove_workspace(workspace_id)
+            return await context.upstream_client.remove_workspace(cast(str, workspace_id))
         if method_key == "list_worktrees":
             return await context.upstream_client.list_worktrees()
         if method_key == "create_worktree":
@@ -357,7 +357,8 @@ async def _invoke_workspace_method() -> Any:
     )
     if upstream_error is not None:
         return upstream_error
-    assert raw_result is not None
+    if raw_result is None:
+        raise UpstreamContractError("OpenCode workspace control returned no payload")
 
     try:
         result = _normalize_response_payload(method_key, raw_result)