[Kyl-Bempah]

Core-first slice of the drift ratchet (XWING-2210): a tool that stops new hand-wiring from creeping in, while leaving existing code alone. "New code follows the platform pattern, and CI stops new drift."

What this adds

• traust_core.v1.checks.ratchet — an AST-based checker (comments/docstrings never count) over six hand-wiring kinds, each mapped to the platform API to use instead:

  Kind	Use instead
  sqlite3.connect / psycopg.connect	SqlUnitOfWork / SqlRepository
  subprocess	ProcessRunner / a ToolProvider
  os.getenv / os.environ	Config / Context
  json.dump / write_text(json…)	ObjectStore / ArtifactPublisher
  datetime.now() / utcnow()	Clock
  requests / httpx / urllib.request	a core provider (SourceProvider, FeedProvider, tool provider)

  CLI: python -m traust_core.v1.checks.ratchet <roots> --baseline .ratchet.json [--reference <base>]. A new hit fails with path:line kind → use <API>; a dropped count fails with "baseline can shrink" so baselines only go down; --reference fails any baseline growth past the base branch's, so a PR cannot raise its own baseline to cover new hand-wiring.

• .ratchet.json — baseline for core's own src. Only the sanctioned wrapper sites (providers/process.py subprocess+env, providers/clock.py clock); core is otherwise clean.

• ratchet job in ci.yml — enforces the baseline on every PR against the base branch's baseline as the reference.

• AGENTS.md — the table above + "new code uses core; touch old code only when cheap."

Scope note

A finite AST checker over the common hand-wiring forms — direct calls, keyword arguments, simple concatenation — not a prohibition on all environment access or JSON persistence (the module docstring says so). Novel forms are caught by review and added here when they appear.

Acceptance criteria (this PR = the traust-core slice)

• AC#1 (six kinds + CLI; test per kind incl. exempt-repository + docstring) ✅
• AC#2 (enforced in CI, shrink-only against the base branch's baseline) ✅
• AC#3 ("baseline can shrink" on a dropped count) ✅
• AC#4 (core baseline; new code starts clean) ✅ for core
• AC#5 (AGENTS table) ✅ for core

Follow-up Work

Consumer rollout lives in the progress-tracker plans Fence traust-engine and Fence traust: a consumer rule set (traust_contracts / traust_core / sqlalchemy / psycopg imports, engine-internal imports, state-file writes; the answer is always the platform API via traust-client), run from CI only (uvx --from traust-core@<sha>), never a runtime dependency. traust-ledger becomes a pure library and gets a "no I/O" rule set or none.

Rebased onto development; the CI reference is now the PR's base branch.

Tests

tests/v1/test_ratchet.py — 16 tests (per kind + exempt-repo + docstring + new-hit/shrink/matching-baseline + keyword/concatenated JSON writes + os.environ.copy + baseline-growth enforcement). Full suite 791 passed; ratchet runs clean against its baseline and the reference; ruff clean.

[smith-xyz]

Great tool — the alias resolution and the shrink-only baseline are exactly right, and the test-per-kind suite is solid. I test-merged this with #7 locally: clean merge (no conflicts, even on AGENTS.md/ci.yml), ratchet green on the combined tree, and the regenerated baseline is byte-identical to the committed one.

Plan: I'll merge #7 first (it lands the platform symbols this tool's "use instead" column points at), then this rebases trivially.

One substantive ask — the http rule, which encodes a direction we're not actually going (my fault; story in the inline comment) — plus a couple of nits. Everything else in the ruleset lines up with the platform API and the layer rules.

[smith-xyz]

This one's on me: XWING-2210's parenthetical "(there is no HTTP client in core)" was me saying I hadn't worked out the REST/HTTP story yet — and it hardened here into "core does no HTTP." That's not the direction.

The ticket's own table says the platform API for requests/httpx is a core provider (SourceProvider, FeedProvider, tool providers), and #7's Layers section is explicit: outbound HTTP lives inside providers (forge, feeds, advisory APIs); inbound is the REST adapter. Concrete case: say we add a VexService that has to fetch data from the VEX feed — that fetch is exactly outbound HTTP in a provider. The pattern working as intended. As written (allowed=()), the ratchet blocks the very thing we want, and the message points at HttpClient, which doesn't exist here either.

Suggested fix — same treatment connect gets:

Rule(
    "http",
    "a core provider (SourceProvider, FeedProvider, tool provider)",
    import_modules=frozenset({"requests", "httpx", "urllib.request"}),
    allowed=("*/providers/*",),  # outbound HTTP lives in providers; add the REST adapter path when it lands
)

That still catches HTTP creeping into services/models/repositories — the actual drift.

[smith-xyz]

Same correction here — per the XWING-2210 table this row should read a core provider (SourceProvider, FeedProvider, tool provider) rather than "none".

Heads-up also: once this rebases onto #7, "see Layers" will point at the new Layers section, which says outbound HTTP is in scope inside providers — the row and the section it cites would disagree with each other.

[smith-xyz]

Tiny nit: env_subscript and json_write are set on the rules but scan_source never reads them — the os.environ[...] subscript and write_text(json.dumps(...)) checks are hardcoded in the loop. Works fine, but the dataclass implies configurability that isn't wired. Either wire them or drop them.

[smith-xyz]

Nit: because __init__.py imports from ratchet and the CI job runs python -m traust_core.v1.checks.ratchet, every CI run logs the "found in sys.modules after import of package… prior to execution" RuntimeWarning. A __main__.py or a lazy import in __init__ silences it.

[Kyl-Bempah]

Sorry for the misunderstanding on how HTTP was meant to work, I believe it should now be updated to more accurately represent the rules.

• http rule: uses your suggested shape: use_instead is "a core provider (SourceProvider, FeedProvider, tool provider)" with allowed=("*/providers/*",) plus the REST-adapter note. New test: import httpx in providers/feeds.py passes and in services/vex.py is flagged (your VexService case).
• AGENTS.md row: now reads "a core provider (SourceProvider, FeedProvider, tool provider)". After the rebase it sits alongside feat(core): land platform API symbols as typed stubs (XWING-2206) #7's Layers section, and the two agree.
• Unwired env_subscript / json_write: removed. The os.environ[...] and write_text(json.dumps(...)) checks stay in the scan loop, so behaviour is unchanged.
• RuntimeWarning: fixed with a lazy module __getattr__ in checks/__init__.py, so from traust_core.v1.checks import scan, … still works and ratchet loads only on first use. Verified with python -W error::RuntimeWarning -m traust_core.v1.checks.ratchet.

Related: the misreading also shaped traust-security/traust#61, so that retry/validate code now lives in core as providers/http.py (HttpFetcher) in #8. #6's providers allowance covers it, so the two can merge in either order.

[chubirka]

Accuracy review: enforcement gaps remain despite passing tests

Reviewed head: e069c79a81451514ea4757908cf2e57bfba926d2. Also tested a clean integration with core main 5be9803.

P2 — "Baselines only shrink" is not enforced across PR changes

ratchet.py:217–234 lets --write-baseline record arbitrary increases. The CI job compares source against the baseline supplied by the same PR; it does not compare that baseline with the target branch's version.

An isolated CLI smoke demonstrated:

1. A new os.getenv() call fails against an empty baseline, exit 1.
2. --write-baseline accepts the new count, exit 0.
3. The unchanged source then passes, exit 0.

The current implementation catches growth relative to a fixed baseline and reports shrinkable counts; it does not ensure that a PR cannot increase its baseline. Please enforce monotonicity against the base branch, or explicitly document an approved baseline-growth escape hatch instead of claiming automatic shrink-only enforcement. Initial baseline bootstrapping can remain a separate operation.

P2 — Ordinary JSON-write and environment-access forms escape detection

ratchet.py:140–157 only recognizes a direct positional json.dumps(...) call as the first write_text argument, and only selected environment-access forms.

These source snippets each returned zero hits:

import json
path.write_text(json.dumps(data) + "\n")

import json
path.write_text(data=json.dumps(data))

import os
settings = os.environ.copy()

Controls using positional path.write_text(json.dumps(data)) and os.getenv("X") were detected. These omissions allow common new hand-wiring to pass the advertised CI gate. Please cover these forms and document the remaining boundaries; a finite AST checker should not be described as a complete prohibition on all environment access or JSON persistence.

Description corrections

The PR body still says core permits no HTTP and that rollout needs a released core. The revised HTTP rule correctly permits providers, and the documented core consumption model uses commit pins rather than requiring a tagged release. Please bring the description into line with the implemented fix and current project guidance.

Exercised verification

• PR-head ratchet/layering checks: 20 tests passed.
• After clean integration with main: 787 unit tests passed; 201 integration tests deselected.
• Actual ratchet CLI passed its committed baseline, including under -W error::RuntimeWarning.
• Ruff lint and format checks passed on the integration tree.
• feat(checks): drift ratchet tool, baseline, CI gate + docs (XWING-2210) #6 and feat(providers): HttpFetcher with retry + feed payload validation (XWING-2223) #8 merge in either order to an identical tree. Their combined unit suite passed (803 passed; 201 integration tests deselected), and the combined ratchet still reports 3 hits across 2 files with the unchanged baseline.

The HTTP allowance, lazy-import warning fix, and ordinary fixed-baseline operation are working. The enforcement gaps above are not covered by the passing suite. PostgreSQL was not exercised. No source changes were made.

Recommendation: changes requested before approval.

[smith-xyz]

@chubirka all three findings addressed in a9753b8 (rebased onto current main first — the branch was 14 commits behind):

1. Baseline growth across PRs — reproduced (new hand-wiring + self-raised baseline passed), then closed: the ratchet gained --reference; CI fetches main's .ratchet.json and fails any count that grew past it. Bootstrap-safe (no reference on main = no check, covering this PR's own first merge). Regression tests: raised baseline fails, shrunk baseline passes.
2. Escaped AST forms — reproduced all three, all now caught: write_text(data=json.dumps(...)) (keyword), write_text(json.dumps(...) + "...") (concatenation), os.environ.copy(). The stricter detection also exposed that the ratchet's own baseline writer uses the concatenated form — now exempted as the sanctioned writer (same mechanism connect uses for repositories). Boundary documented in the module docstring: a finite checker over common forms, not a prohibition.
3. Description corrections — PR body updated: the HTTP row names the core providers (matching the rule Kyl already fixed), and the rollout section says commit pins, not a tagged release.

Gates on this head: lint clean, 791 unit, 200 Postgres integration, coverage 93.8%; ratchet green against baseline + reference on the rebased tree (3 hits / 2 files, byte-identical — the table-repository refactor added zero hand-wiring).

[chubirka]

/lgtm