Files
svcforge/tests/unit/test_protocol_conformance.py
Nguyen Minh Phuc c76154aeaa
ci / lint (push) Successful in 34s
ci / unit (push) Successful in 1m41s
ci / types (push) Successful in 1m41s
ci / dockerfile (push) Successful in 18s
ci / security (push) Successful in 1m27s
ci / chart (push) Failing after 1m11s
ci / integration (push) Successful in 1m10s
ci / image (api) (push) Has been skipped
ci / image (reconciler) (push) Has been skipped
ci / image (worker) (push) Has been skipped
ci / bump (push) Has been skipped
review: fix 26 findings from a 4-agent audit
CORRECTNESS
- lost-lease race: complete()/fail() did not check ownership, so a worker whose
  lease expired could mark a task done while another worker was running it, or
  requeue a task someone else owned. Reproduced, fixed with a CAS on
  (state, locked_by), pinned by two regression tests.
- worker died on report failure: _run_one's docstring claimed no exception
  escapes the TaskGroup; fail()/complete() were outside the guarded block, so a
  DB blip cancelled every sibling provision on the pod.
- claim query used an INNER join, which could strand a just-claimed task and
  report 'queue empty'. LEFT join.
- InstanceRepo.set_error bypassed the state machine and had no callers. Deleted.
- handle_deprovision ignored its CAS result, so a wrong-state instance kept a
  dangling endpoint and got re-provisioned by the drift check 60s later.
- handle_verify re-notified on every retry: five pages for one halt.

DEPLOY-BREAKING
- the migration Job could never succeed: no Dockerfile copied migrations/, and
  migrate.py resolved the path relative to the source tree, which only works for
  an editable install. Added COPY + SVCFORGE_MIGRATIONS_DIR.
- ServiceMonitor selector did not match the Service: API metrics never scraped.
- SvcforgeReconcilerStale fired permanently from every pod, because the gauge is
  module-level and every service exports it as 0. Scoped to the reconciler job.
- SvcforgeTaskFailed latched forever on a monotonic counter. Now increase()[15m].
- the digest guard accepted the all-zeros placeholder.
- worker terminationGracePeriodSeconds was 60s against a 600s helm timeout.

DEAD CODE THAT SHOULD NOT HAVE BEEN
- adapters/k8s.py was never called, so tenant namespaces were never created and
  the first provision for a new team would fail. Wired into handle_provision.
- adapters/redis.py was never imported by any service. Rate limiting is now wired
  into the API, failing open.
- Settings.check_production() had no callers. Given an explicit environment and
  called from every entrypoint.

OBSERVABILITY
- the API never called obs.setup(): no JSON logs, no trace correlation, log_json
  silently inert.
- LogNotifier's structured fields were discarded by the stdlib->structlog bridge.
- bind_task_context cleared the 'service' binding for the life of every task.
- split tasks_failed into task_attempts_failed and tasks_dead_lettered.

SECURITY
- trivy correctly blocked the worker/reconciler images: helm 3.16.2 and kubectl
  1.31.2 carry CRITICAL Go stdlib CVEs. Bumped to helm 3.21.3 and kubectl 1.35.3,
  which also closes a four-minor skew against the v1.35.3 cluster.

TESTS THAT COULD NOT FAIL
- the concurrency cap test passed on a fully serial worker.
- the alert/metric cross-check asserted a hardcoded list instead of reading the
  chart, so it could not catch a rename on the chart side.
- fixed OTel tracer-provider pollution between test files.

DOCS
- ARCHITECTURE.md: mermaid diagrams, user stories, and the helm-vs-ArgoCD
  guarantee (verified with --dry-run=server).
- AGENTS.md + CLAUDE.md.
- prose sweep for back-and-forth phrasing across 19 files.
2026-07-18 12:13:49 +00:00

110 lines
3.7 KiB
Python

"""Every Protocol, with both of its implementations, checked by mypy.
These functions have no assertions and cannot fail at runtime — the check happens under
`mypy --strict`. If FakeProvisioner drifts from Provisioner (a renamed parameter, a changed
return type), the type-check fails here rather than the fake quietly diverging from the real
adapter and the worker's fast tests proving nothing about production.
The test bodies exist so pytest runs the imports too: a Protocol satisfied statically but
broken at import time is still broken.
"""
from __future__ import annotations
from pathlib import Path
from svcforge_core.adapters.clock import Clock, SystemClock
from svcforge_core.adapters.helm import HelmProvisioner, Provisioner
from svcforge_core.adapters.notify import LogNotifier, Notifier, WebhookNotifier
from svcforge_core.adapters.redis import (
IdempotencyStore,
IdempotencyStoreProto,
InstanceCache,
InstanceCacheProto,
RateLimiter,
RateLimiterProto,
make_redis,
)
from svcforge_core.settings import Settings
from tests.fakes import (
FakeClock,
FakeIdempotencyStore,
FakeInstanceCache,
FakeNotifier,
FakeProvisioner,
FakeRateLimiter,
)
def take(p: Provisioner) -> None:
"""Accepts anything structurally a Provisioner. The whole assertion is the annotation."""
def take_clock(c: Clock) -> None: ...
def take_notifier(n: Notifier) -> None: ...
def test_provisioner_implementations_conform() -> None:
take(HelmProvisioner(kubeconfig=Path("/dev/null")))
take(FakeProvisioner())
print("ok")
def test_clock_implementations_conform() -> None:
from datetime import UTC, datetime
take_clock(SystemClock())
take_clock(FakeClock(start=datetime(2026, 1, 1, tzinfo=UTC)))
print("ok")
def test_notifier_implementations_conform() -> None:
take_notifier(LogNotifier())
take_notifier(WebhookNotifier(url="https://example.invalid/hook"))
take_notifier(FakeNotifier())
print("ok")
def take_limiter(rl: RateLimiterProto) -> None: ...
def take_idempotency(store: IdempotencyStoreProto) -> None: ...
def take_cache(c: InstanceCacheProto) -> None: ...
def test_redis_implementations_conform() -> None:
"""A client at a closed port is still a Redis. Nothing here connects — no commands billed.
That is the module's thesis restated as a fixture, rather than a trick to keep the
test fast. Every one of these classes has to be constructible and callable with Redis
unreachable, because that is the state they are designed for.
"""
from datetime import UTC, datetime
settings = Settings(
# pydantic parses these strings into PostgresDsn/RedisDsn at runtime; the
# annotation names the parsed type, so the ignores sit on the arguments.
pg_dsn="postgresql://unused:unused@127.0.0.1:5432/unused", # type: ignore[arg-type]
redis_dsn="redis://127.0.0.1:1/0", # type: ignore[arg-type]
)
r = make_redis(settings)
assert r is not None
take_limiter(RateLimiter(r, limit=10, window_s=60))
take_limiter(FakeRateLimiter(10, 60, FakeClock(start=datetime(2026, 1, 1, tzinfo=UTC))))
take_idempotency(IdempotencyStore(r))
take_idempotency(FakeIdempotencyStore())
take_cache(InstanceCache(r))
take_cache(FakeInstanceCache())
# redis_dsn=None EXPLICITLY. Omitting it does not mean "unset": pydantic-settings
# reads SVCFORGE_REDIS_DSN from the environment, so on any machine that has the real
# DSN exported this assertion sees a live Upstash client and fails — a green test that
# depends on your shell being empty is not a test.
assert make_redis(Settings(pg_dsn=settings.pg_dsn, redis_dsn=None)) is None
print("ok")