feat: Add read-only AssessTables RPC

[erezrokah]

Adds an optional, read-only AssessTables RPC to the v3 plugin service that returns per-table and per-column schema impact findings, so destinations can report how they would apply a schema change without writing anything.

Fixes https://linear.app/env-zero/issue/ATH-935/add-a-read-only-destination-assessment-rpc-to-plugin-pb

[cloudquerydaniel]

VERDICT

✅ SAFE — NO DEFECTS FOUND (proto compiles and is wire-compatible; no runtime exists in this repo to exercise the RPC)

0 ❌ failed · 4 ✅ passed · 0 🚫 blocked
Coverage: 3 of 3 important checks ran · not run: none
Layers: implied api · reached api · not reached: none · n/a: none

Metadata: target local buf build of the PR's proto (rung 3; no preview/deploy exists, repo is a proto-only library) · commit dc2f4ab · playbook api.md (spec-diff step; no auth model, so wrong-caller steps do not apply) · qa run: none · agent docs: none found · mode on CI

Assumed: CI=true is set, so CI mode. PR row: a PR number, tested on its head commit. Test-run pass covers a single file; no other PRs comments/reviews exist.

LEADS

CLAIMS: adds an optional read-only AssessTables RPC returning per-table/column findings; plugins that don't support it return Unimplemented.
WORRIES: none raised (no reviews, no comments; the Linear ticket ATH-935 could not be read from this run, so it is a LEAD only).
IMPORTANT:

1. The proto still compiles with the new RPC and messages.
2. The change is wire/source-compatible with the base (no removed/renamed field, no renumbering).
3. Callers of the changed service are ruled out or covered (older plugins and clients).

CHECKS

P	Layer	Check	How	Result
P0	api	proto compiles with new RPC	buf build (v1.50.0) on head plugin/v3/plugin.proto	reached — ✅ exit 0, no errors
P0		no breaking change vs base	buf breaking --against base.bin (base = HEAD~1 proto)	✅ exit 0, no output
P0		additive-only on the wire	diff git diff HEAD~1: only added rpc AssessTables and new message AssessTables; no existing line changed	✅ 0 removed lines in the diff
P1		enum value names do not collide	buf build (proto3 enum values are scoped to enclosing message AssessTables; CATEGORY_* is unique)	✅ build clean
P1		style lint	buf lint on head vs base	✅ new findings only the repo's existing Request/Response naming convention (2 lines for AssessTables, same as the 12 existing RPCs); not a defect
P1		CI jobs	gh pr view --json statusCheckRollup	covered-by-CI — CodeQL, Analyze (actions) SUCCESS; kodiakhq NEUTRAL. CI has no proto-compile job
P1		deploy logs	repo builds nothing and deploys nothing	⏭️ not run — no deploy or log exists

FINDINGS

🔴 BROKEN

None.

🟠 RISKY

None.

🟡 NOTE

• AssessTables.Response.tables carries only table_name; the request has no names, only TablePair schema bytes. A client must unmarshal each schema to match findings to requests, and a rename (old name ≠ new name) has no defined key. Worth a comment in the proto saying how findings map to pairs. Inferred — confirm with author.
• Category is shared by tables and columns, so CATEGORY_TABLE_REMOVED and CATEGORY_FILE_SCHEMA_CHANGED are valid values on a ColumnFinding with no meaning. Documentation gap only.
• TablePair marks added/removed tables by an empty bytes field, so a pair with both empty is representable and undefined. Say which error the plugin returns.

RULED OUT

• Existing plugins/SDK clients: the RPC is a new method on service Plugin and a new top-level message. Old servers answer Unimplemented, which the proto comment documents. buf breaking exit 0 supports it. Command: buf breaking --against base.bin.
• Other messages' names: AssessTables is the only new top-level name; grep -n "message AssessTables" plugin/v3/plugin.proto returns 1 line.

NOT COVERED

• Behaviour of any server or client: the repo has only the .proto; generated code and implementations live in plugin-sdk and are outside this PR.
• Ticket ATH-935's acceptance criteria: not readable from this run.

PROPOSED

• In plugin-sdk, a test that a server without AssessTables returns codes.Unimplemented.

FOR THE REPO

• ## QA run is absent. Draft: Target — none, proto-only library with no deploy; run buf build and buf breaking --against <base> on plugin/v3/plugin.proto. Trap — no buf/protoc is installed; GOBIN=/tmp/x go install github.com/bufbuild/buf/cmd/buf@v1.50.0 works. Trap — buf lint fails on the existing Request/Response naming, so only compare against base.

DETAILS

THE PLAYBOOK QUESTION, ANSWERED

Wrong caller: the proto defines no identity model, roles or tenants, so there is no wrong caller to refuse; access checks are NOT COVERED by design. The contract (step 2) was compared: base vs head via buf breaking, exit 0.

EXISTING TESTS RUN

None exist in the repo (no tests, no CI proto job).

TESTS WRITTEN FOR THIS RUN

None; the compiler and breaking check are the probes. Head: buf build exit 0, buf breaking exit 0. Base: buf build exit 0.

GENERATED DIFFS

None checked in; the repo holds no generated code.

CHECKS AGAINST THE TARGET

Target is a local buf build in /tmp/qa40w (removed at the end), not a deploy. Config used: version: v2, module ..

[cloudquerydaniel]

Summary

Adds an optional, read-only AssessTables RPC to the v3 plugin service. Destinations return per-table and per-column schema-change findings without writing anything. Only plugin/v3/plugin.proto changes (+53 lines); the new RPC is additive, so existing clients are unaffected.

Review

✅ SAFE TO MERGE

Additive proto change, no field renumbering and no existing message touched. Worth a look: the finding shape is loose (see below), and it is hard to change once plugins implement it.

Testing: none needed; the repo has no tests or written testing rules. Generated code lives in other repos.

Nice to have

• plugin/v3/plugin.proto:262 — Category mixes table-only values (TABLE_REMOVED, FILE_SCHEMA_CHANGED) with column values and is reused for both levels, so a column can legally carry TABLE_REMOVED. Split into table and column enums, or document which values apply where.
• plugin/v3/plugin.proto:277 — safe_mode_behavior / forced_mode_behavior are free-form strings, and "safe mode" is defined nowhere; the request only has migrate_force. Name the modes in a comment, or return the behavior for the requested mode only.
• plugin/v3/plugin.proto:285 — ColumnFinding and TableFinding repeat category, both behavior strings and evidence. Fold them into one shared Finding message embedded in each.
• plugin/v3/plugin.proto:289 — coverage_incomplete + coverage_incomplete_reason can be one field: a non-empty reason means incomplete. Drops a field that can disagree with its pair.
• plugin/v3/plugin.proto:254 — TablePair comments say "empty when added/removed". Using bytes emptiness as the signal can't tell "absent" from "failed to marshal"; match how Write.MessageMigrateTable.table is used, or add optional.

No comments in the diff restate code. No existing shared type to reuse in the repo beyond the Write.MessageMigrateTable pattern noted above.

Files reviewed (1)

File	Outcome
plugin/v3/plugin.proto	🔵 shape suggestions only

_{Stack: protobuf · kinds reviewed: API schema · repo guidance: none found (no AGENTS.md); README.md
Last reviewed commit: dc2f4ab · reviewed 2026-10-06 UTC}

[erezrokah]

Review follow-up (44c4c8e):

• fixed Category scope: documented that TABLE_REMOVED applies to tables only.
• fixed Safe/forced mode: documented as migrate_force false/true. Both behaviors stay in the response so a report can show both without two calls.
• fixed Coverage: replaced coverage_incomplete + reason with one incomplete_coverage_reason field (non-empty = incomplete).
• answered Shared Finding message: kept flat. Table and column findings differ (columns have types, tables have coverage and nested columns), and flat fields are simpler for every destination to fill.
• answered optional on TablePair bytes: a marshalled Arrow schema is never empty, so empty bytes only mean absent. This matches the existing bytes table fields.
• QA report: no defects, nothing to do.

Regenerated bindings pushed to cloudquery/plugin-pb-go#703, SDK updated in cloudquery/plugin-sdk#2618.

🤖 Addressed by Claude Code