ci(sdk): add proto drift detection and sync notifications by Ygnas · Pull Request #3123 · NVIDIA/OpenShell

Ygnas · 2026-09-02T08:41:59Z

Summary

Add daily CI workflow that detects proto drift in Go and TypeScript SDKs, runs build checks when drift is found, and auto-manages GitHub issues (create/update on breakage, close on resolution).

Related Issue

Closes #2825

Changes

.github/workflows/sdk-sync-dashboard.yml — New workflow: runs daily at 06:00 UTC, checks both SDKs in a single job, manages drift issues per-SDK via matrix strategy
tasks/go.toml — Two new tasks: go:proto:drift (generates fresh protos to a tmpdir, diffs against committed files, outputs JSON report) and go:proto:build-check (delegates to shared build check script)
tasks/scripts/sdk_build_check.sh — Generic step runner: executes a sequence of mise tasks, captures logs, outputs structured pass/fail JSON
tasks/scripts/sdk_sync.py — Issue management CLI: generates issue bodies with drift tables, build logs, fix commands, and an agent-consumable prompt; deduplicates by label

Example generated issue

This example was generated from a real test run, producing Go stub drift and a converter build failure. The build log is shortened for readability.

Example Go SDK drift issue

Proto Drift Report

Summary: 1 file(s) drifted

File	Status	Diff Lines
`openshellv1/openshell.pb.go`	modified	36

Build Log

Failed step: build

[go:build] $ go build ./...
# github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/internal/converter
openshell/v1/internal/converter/sandbox.go:58:21: spec.GetLogLevel undefined
  (type *openshellv1.SandboxSpec has no field or method GetLogLevel)
openshell/v1/internal/converter/sandbox.go:201:3: unknown field LogLevel in struct literal of type openshellv1.SandboxSpec
[go:build] ERROR task failed

Fix Commands

mise run go:proto:gen    # Regenerate bindings
mise run go:build        # Verify build
mise run go:test         # Run tests

Agent Instructions

This section is a ready-to-consume prompt for an AI agent. Copy it into your agent to produce a fix PR.

Agent prompt (click to expand)

Fix proto drift in the Go SDK.

Context

The root proto/ directory has changed and the Go SDK's generated bindings are out of sync. The drifted files are: openshellv1/openshell.pb.go.

The SDK build fails at the build step after regenerating protos. The build log above shows the exact error. Your job is to fix the Go SDK code so it compiles and passes tests with the updated protos.

Steps

Regenerate bindings: Run mise run go:proto:gen to regenerate language-specific bindings from the updated protos.
Fix compilation errors: Read the build log above. Update the SDK source code to handle new/changed/removed proto fields:
- sdk/go/openshell/v1/internal/converter/
- sdk/go/openshell/v1/types/
- sdk/go/openshell/v1/
Fix test failures: Update tests that assert on proto types that changed shape.
Verify: Run mise run go:build and mise run go:test until both pass.
Create a PR: Commit all changes and create a PR referencing this issue.

Scope

Only modify files under sdk/go/. Do not change root proto/ files.
Do not change the proto definitions. Adapt the SDK to match them.
Keep changes minimal: only fix what the proto changes broke.

Testing

mise run go:proto:drift — outputs valid JSON when in sync (exit 0) and detects drift when a proto file is modified (exit 1, correct JSON)
mise run go:proto:build-check — reports success when gen/build/test pass
mise run sdk:ts:proto:drift — outputs valid JSON when in sync
mise run sdk:ts:proto:build-check — reports success when gen/typecheck/test pass
sdk_sync.py generate_issue_body — produces correct issue body for both Go and TypeScript
mise run pre-commit passes
Verify workflow runs successfully via workflow_dispatch

Checklist

Follows Conventional Commits
Commits are signed off (DCO)
Architecture docs updated (if applicable)

cc @rhuss

copy-pr-bot · 2026-09-02T08:42:02Z

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

rhuss

A first round of human review. Looks good in general, some minor comments inline.

I hand over now to my review agents for additional findings.

rhuss · 2026-09-03T08:46:13Z

+          - name: go
+            drift_task: "go:proto:drift"
+          - name: typescript
+            drift_task: "sdk:ts:proto:drift"


Why are the tasks are named following a different scheme ? I would propose to use also sdk:go:proto:drift for the naming of the golang drift task.

The new task follows the existing Go task namespace (go:ci, go:proto:gen, and go:proto:check), while TypeScript currently uses sdk:ts:*. As discussed, migrating all Go tasks to sdk:go:* would improve monorepo consistency, but that broader rename is better handled in a follow-up PR. I’m keeping go:proto:drift consistent with the current Go tasks here.

Fair point. The go:* namespace is already established for all Go tasks, so renaming just the drift tasks to sdk:go:* would create an inconsistency within the Go namespace. A full rename can be a follow-up if the project decides to unify under sdk:*. Resolving.

rhuss · 2026-09-03T08:54:46Z

 """
 hide = true
+
+["go:proto:drift"]


as said above, I would align the naming convention along side the typescript task (and add a sdk: prefix)

Same as #3123 (comment)

Agreed, same reasoning as above. Resolving.

rhuss · 2026-09-03T08:55:48Z

+    return subprocess.CompletedProcess([], returncode, stdout=stdout, stderr=stderr)
+
+
+class TestGenerateIssueBody:


The tests only check for golang based drifts. Would it make sense to add some typescript-based tests ?

There is already test_typescript_sdk_issue_body which checks TypeScript commands, source path and gitignored generated files. Because TypeScript generated files are not committed, there is no file drift list like for Go. So I think the TypeScript-specific case is already covered and no extra test is needed in this PR.

Makes sense. The TS drift model (regen + typecheck) is fundamentally different from Go's file-diff approach, so there's less to unit test at the drift detection layer. The existing test_typescript_sdk_issue_body covers the TS-specific issue body content. Resolving.

rhuss

cc-review Summary

What Went Well

Clean separation of concerns: Drift detection in per-SDK mise tasks, issue management in standalone Python CLI, workflow orchestration in YAML. Each layer independently testable and replaceable.
TypeScript drift detection correctly adapts to the gitignored-stubs model by using regen-then-typecheck rather than file diffing (tasks/typescript.toml:70).
Agent-consumable prompt in issue bodies (sdk_sync.py:100-167): well-structured prompt with context, steps, and scope constraints inside a collapsible <details> block, aligned with the project's agent-first identity.
Issue deduplication by label prevents daily cron drift issues from piling up (sdk_sync.py:248-308).
Robust error handling in workflows: || true on drift commands, jq -e validation, ::warning::/::error:: annotations with stderr capture for debugging.

Findings

Severity	File	Description	Source
Important	`tasks/typescript.toml:78`	TS drift summary misattributes proto-gen failure as typecheck	correctness
Important	`sdk-sync-dashboard.yml:25`	No timeout-minutes on CI jobs (6h default)	production
Important	`tasks/go.toml:180`	go:proto:drift duplicates ~70% of go:proto:check	architecture
Minor	`sdk-proto-check.yml:78`	Fixed heredoc delimiter enables output injection	security
Minor	`tasks/go.toml:209`	NDJSON_FILE variable actually contains TSV	architecture
Minor	`tasks/go.toml:239`	Exit code re-derived from file already consumed by jq	architecture
Minor	`tasks/go.toml:209`	NDJSON temp file not in EXIT trap	correctness
Minor	`sdk_sync.py:220`	_ensure_label swallows label creation failure	correctness
Minor	`sdk_sync.py:190`	subprocess.run calls have no timeout	production
Minor	`sdk-sync-dashboard.yml:46`	Dashboard extensibility requires YAML changes per SDK	goal-alignment

Notable Observations

File	Description	Source
`sdk_sync_test.py`	manage_issue error returns have zero coverage	test-quality
`sdk_sync_test.py`	CLI entrypoint and exit code logic untested	test-quality
`sdk_sync_test.py`	Mocked _run_cmd args never inspected	test-quality
`sdk_sync_test.py`	_find_open_issue JSON error handling untested	test-quality
`sdk_sync_test.py`	No-build-report agent prompt path unverified	test-quality
(PR description)	PR Changes section omits 3 files including PR workflow	goal-alignment

Review Details

Findings posted: 16 (3 Important, 7 Minor, 6 Notable)
Findings reviewed and not posted: 0
Gate outcome: FAIL (3 Important findings)
Participating agents: correctness, architecture, security, production, test-quality, goal-alignment

Add automated proto drift detection for Go and TypeScript SDKs with issue-based notifications when SDK builds break due to proto changes. Signed-off-by: Ignas Baranauskas <ibaranau@redhat.com>

Signed-off-by: Ignas Baranauskas <ibaranau@redhat.com>

rhuss

cc-review Re-Review Summary

All 10 findings from the initial review have been addressed in commit 051d7f4. Re-reviewed all 8 new/changed files and verified each fix.

Fixes verified

Original Finding	Fix
TS drift misattribution	Steps separated, distinct error messages
No timeout-minutes	All jobs now have appropriate timeouts
go:proto:drift duplication	Shared `go_proto_check.sh` with `--output-format`
Fixed heredoc delimiter	Randomized with `openssl rand -hex 16`
NDJSON_FILE misleading name	Renamed to `RESULTS_FILE`
Exit code re-derivation	Derived from jq output directly
Temp file leak	All temp files in EXIT trap
_ensure_label swallows failure	Now raises RuntimeError with details
No subprocess timeout	60s default with TimeoutExpired handling
Dashboard extensibility	Config-driven matrix + artifact-based data passing

New additions reviewed (clean)

tasks/scripts/go_proto_check.sh: Shared script, both output modes correct
tasks/sdk-sync-config.json: Central config consumed by workflows + Python
3 new tests covering timeout, label failure, and structured errors
Dashboard workflow refactored to artifact-based approach

Gate outcome: PASS
CodeRabbit: service unavailable (Anthropic API partial outage)

rhuss · 2026-09-03T14:22:20Z

@drew @mrunalp @derekwaynecarr @elezar Pinging for review on this PR. It adds automated proto drift detection for the Go and TypeScript SDKs with two workflows:

PR-triggered (sdk-proto-check.yml): Annotates proto-touching PRs with drift warnings so contributors see which SDKs are affected. Non-blocking, informational.

Daily cron (sdk-sync-dashboard.yml): Runs drift detection and, if the SDK build breaks, auto-creates a GitHub issue per SDK with fix commands and an agent-consumable prompt (so gator or a contributor's agent can pick it up directly). Auto-closes when drift resolves.

Where it adds value:

TypeScript SDK: This is where the real gap was. There was no drift detection at all. The PR adds a regen-then-typecheck approach that catches when handwritten TS code no longer compiles against updated proto definitions.
Go SDK: The stub-level drift detection is largely covered by the existing go:proto:check CI task and coverage_test.go (which already fails on unmapped converter fields). The new value for Go is the issue lifecycle automation and the extensible infrastructure. The deeper "converter coverage analysis" (detecting fields present in protos but absent from SDK type mappings) is a stated follow-up in #2825.
Extensibility: Adding a new SDK is a config entry in tasks/sdk-sync-config.json plus a mise task. Both workflows consume the config via matrix strategy.

The review went through two rounds and all findings have been addressed. Ready for merge from my side.

Ygnas requested review from a team, derekwaynecarr, mrunalp and sjenning as code owners September 2, 2026 08:42

rhuss suggested changes Sep 3, 2026

View reviewed changes

Ygnas added 2 commits September 3, 2026 13:47

ci(sdk): add proto drift detection and sync notifications

978f850

Add automated proto drift detection for Go and TypeScript SDKs with issue-based notifications when SDK builds break due to proto changes. Signed-off-by: Ignas Baranauskas <ibaranau@redhat.com>

fix(ci): address sdk proto sync review feedback

051d7f4

Signed-off-by: Ignas Baranauskas <ibaranau@redhat.com>

Ygnas force-pushed the feat/sdk-proto-sync-ci branch from 6c8a903 to 051d7f4 Compare September 3, 2026 12:47

Ygnas requested a review from rhuss September 3, 2026 13:54

rhuss approved these changes Sep 3, 2026

View reviewed changes

rhuss mentioned this pull request Sep 3, 2026

feat(ci): automated proto drift detection and SDK sync notifications #2825

Open

6 tasks

		return subprocess.CompletedProcess([], returncode, stdout=stdout, stderr=stderr)


		class TestGenerateIssueBody:

Uh oh!

Conversation

Ygnas commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Related Issue

Changes

Example generated issue

Proto Drift Report

Build Log

Fix Commands

Agent Instructions

Context

Steps

Scope

Testing

Checklist

Uh oh!

copy-pr-bot Bot commented Sep 2, 2026

Uh oh!

rhuss left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rhuss left a comment

Choose a reason for hiding this comment

cc-review Summary

What Went Well

Findings

Notable Observations

Review Details

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rhuss left a comment

Choose a reason for hiding this comment

cc-review Re-Review Summary

Fixes verified

New additions reviewed (clean)

Uh oh!

rhuss commented Sep 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Ygnas commented Sep 2, 2026 •

edited

Loading