feat(sdk/go): three-layer proto-to-API coverage checks (RPC methods, converter wiring, message auto-discovery)
还没有人认领这个 Issue。
评估
- 难度
- 5/5
- 预计耗时
- 一周以上
- 新手友好度
- 35/100
- Issue 类型
- 功能
- 描述清晰度
- 基本清楚
- 活跃度
- 活跃
- 技术栈
- github-actions, go
- 领域
- api, backend-api-design, ci-cd, testing-qa
调研方向
从 sdk/go/openshell/v1/internal/converter/coverage_test.go 开始,然后检查 OpenShell 和 Inference 服务描述符以及 ClientInterface 的子客户端接口。定义 reflection 应如何发现消息、RPC 和 converter 输出,包括有充分理由跳过的例外。完成的标准是报告全部三个缺口,同时字段覆盖率仍然必须满足,并且通过提议的工作流或 #3123 基础设施使 RPC 和 wiring 检查不阻塞。
由索引模型根据 Issue 内容生成。
描述
User Story
As a Go SDK maintainer, I want automated detection when new proto RPCs or message types are added but not yet exposed through the curated SDK API, so that coverage gaps are visible without manual tracking.
As a proto contributor, I want these checks to be non-blocking on my PR, so that I can evolve the API without being gated by SDK implementation work.
Problem Statement
The Go SDK has a three-layer chain from proto definitions to public API: proto fields → converter → curated client. Today, only the first link is checked.
coverage_test.go uses protobuf reflection to verify that all fields on specific message types are handled by converters. This catches new fields added to existing messages. However:
- New message types are not auto-discovered. Each message needs a hand-written test function with a
handledfield set. A new RPC with new request/response message types has no coverage test until someone writes one. - New RPC methods are not checked. A new service RPC in the proto can land with regenerated stubs, pass CI, and never be wrapped in the curated
ClientInterface. - Orphaned converters are not detected. A converter function can exist without any curated client method referencing its output type.
Because the Go SDK commits generated stubs, every proto-changing PR touches sdk/go/, which triggers coverage_test.go. This is the right behavior for field coverage (narrow, mechanical fix). But extending it to RPC-level coverage would block proto contributors on full SDK implementation, which #2825 explicitly rejected.
Impact / Why This Matters
Current behavior: A PR adding a new RPC (e.g., PauseSandbox) regenerates Go stubs, passes all CI checks, and merges. The curated Go SDK silently omits the new capability. SDK consumers discover missing methods through trial and error.
Current workaround: SDK maintainers manually monitor proto changes and audit the curated client surface. The coverage_test.go catches field gaps on known messages but not structural gaps (missing RPCs, missing converters).
Why the workaround is insufficient: As the API surface grows and more contributors change proto definitions, manual tracking doesn't scale. The existing field-level coverage test created a false sense of completeness: it catches one class of gap but not the other two.
Proposed Design
Add two new reflection-based coverage checks and extend auto-discovery for the existing one.
Layer 1 (extend existing): Auto-discover message types
Instead of requiring a hand-written TestConverterCoversAllProtoFields_* function per message type, scan proto service descriptors to collect all request/response message types (and their nested messages) automatically. Compare against the set of messages that have converter coverage. Flag any message type used by a service RPC that has no converter coverage test.
Layer 2 (new): RPC method coverage
Use protoreflect to enumerate all RPC methods across the OpenShell and Inference service descriptors. Compare against methods on ClientInterface and its sub-client interfaces (SandboxClient, ProviderClient, etc.). Flag any RPC with no corresponding curated client method.
Layer 3 (new): Converter-to-client wiring
Use reflect to scan all ClientInterface method signatures (arguments and return types). Collect every domain type referenced. Compare against the set of domain types produced by converter functions. Flag any converter output type not referenced by a client method signature.
All three layers follow the same self-maintaining pattern: reflection-based scanning with a skipped set for intentional exceptions (e.g., internal-only RPCs, types exposed only through raw). No manual lists to keep in sync.
Enforcement model:
| Check | Enforcement | Rationale |
|---|---|---|
| Field coverage on existing messages | Hard (blocks PR) | Narrow, mechanical fix. Already works this way. |
| RPC method coverage | Soft (non-blocking) | Requires design decisions about the SDK API surface. |
| Converter-to-client wiring | Soft (non-blocking) | Catches orphaned converters, lower urgency. |
Soft enforcement options (one or both):
- A separate GitHub Actions job marked
continue-on-error: true(visible on PRs but non-blocking) - Integration with the daily cron from #3123 (creates issues with agent prompts when gaps are found)
Acceptance Criteria
- Auto-discovery: new proto message types used by service RPCs are detected without hand-written test functions
- RPC coverage: new service RPC methods not wrapped in
ClientInterfaceare detected - Converter wiring: converter output types not referenced by client method signatures are detected
- All checks use a
skippedset for intentional exceptions with justification comments - Field-level coverage remains a hard gate (existing behavior unchanged)
- RPC and converter checks are non-blocking on proto-changing PRs
- Gaps detected by soft checks create actionable issues (via #3123 infrastructure or separate workflow)
Alternatives Considered
Hard gate for all layers: Rejected. Because Go stubs are committed, every proto-changing PR touches sdk/go/, so a hard RPC coverage gate would block proto contributors on SDK implementation. This contradicts the project's decision in #2825.
External linter instead of test-time reflection: Possible but adds toolchain complexity. The reflection-based approach matches the existing coverage_test.go pattern and requires no new dependencies.
Skip Go-specific checks, rely on daily cron only: The daily cron from #3123 detects stub-level drift but not converter or RPC gaps. The reflection-based checks provide deeper coverage that the cron infrastructure alone cannot.
Related
- #2825 (proto drift detection and sync notifications, covers stub-level drift)
- #3123 (PR implementing #2825, provides the daily cron and issue lifecycle infrastructure)
sdk/go/openshell/v1/internal/converter/coverage_test.go(existing field-level coverage)
- 主要语言
- Rust
- 星标
- 8.7k
- 派生
- 1.3k
- 平均合并
- 2 天 6 小时
- 30 天内合并 PR
- 297
贡献指南
从这里开始
- 先读完整个 Issue,再读项目的贡献指南。
- 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
- Fork 仓库,在一个分支上完成修改。
- 提交 Pull Request,并在描述里引用这个 Issue 编号。
NVIDIA/OpenShell 的其他 Issue
-
area:docs
难度 1/5 1 小时以内 新手友好度 88/100
-
state:triage-needed
难度 2/5 1-3 小时 新手友好度 82/100
-
area:cli state:validated
难度 2/5 1-3 小时 新手友好度 72/100
-
state:triage-needed
难度 1/5 1 小时以内 新手友好度 90/100
-
area:build spike state:review-ready state:stale
难度 2/5 半天 新手友好度 68/100
相似的 Issue
-
bug CLI custom-model
难度 2/5 1-3 小时 新手友好度 75/100
-
难度 2/5 1-3 小时 新手友好度 75/100
rust-bitcoin/rust-bitcoin#6930 · 1 条评论 ·
-
难度 2/5 1-3 小时 新手友好度 75/100
-
难度 2/5 1-3 小时 新手友好度 75/100
fulcrumgenomics/ferro-hgvs#2251 ·
-
A-allocators A-docs C-enhancement T-libs
难度 2/5 1-3 小时 新手友好度 75/100