feat(tern): one deployment correlates to exactly one remote apply - #1060
Draft
aparajon wants to merge 4 commits into
Draft
feat(tern): one deployment correlates to exactly one remote apply#1060aparajon wants to merge 4 commits into
aparajon wants to merge 4 commits into
Conversation
…ion echo Operation-scoped remote dispatches now share one idempotency key per deployment and generation instead of minting one key per operation, so a deployment's sibling operations land on a single data-plane apply — the first dispatch creates it and each sibling attaches its own operation. Because the shared apply answers many operations under one key, an accepted response is only trusted when it echoes the operation key the request's shape derives to; a response without the right echo (most often a data plane that predates sibling-operation attach and would alias every sibling to the first operation) is refused, the dispatch fails closed, and a counter fires for the operator. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
All operations of a deployment attach into the deployment's single data-plane apply, so they all record the same remote apply id. persistRemoteApplyID now fails closed when a dispatch result would give a deployment a second remote apply, DeploymentRemoteApplyID resolves the shared id for read paths, and the refusal is countable via schemabot.remote_apply_deployment_id_conflict_total. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…red-external-id # Conflicts: # pkg/metrics/README.md # pkg/metrics/metrics.go # pkg/tern/grpc_client.go # pkg/tern/grpc_client_test.go
Contributor
There was a problem hiding this comment.
Pull request overview
This PR strengthens SchemaBot’s remote (gRPC) apply correlation model by enforcing the invariant that a single deployment must map to exactly one remote data-plane apply ID, failing closed when a dispatch response would split a deployment across multiple remote applies. This improves operator triage by ensuring there’s a single remote apply identifier to correlate across control-plane and data-plane logs/storage.
Changes:
- Add a write-path guard (
guardDeploymentRemoteApplyID) that rejects persisting a remote apply ID when sibling operations for the same deployment already recorded a different ID (or already disagree). - Introduce shared storage helpers (
ApplyOperation.RemoteApplyID()+storage.DeploymentRemoteApplyID) to consistently resolve the remote apply ID (including legacyengine_resume_contextfallback) and fail closed on disagreement. - Add a new metric (
schemabot.remote_apply_deployment_id_conflict_total) and document the operator action in the metrics README.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/tern/grpc_client.go | Enforces deployment-level single-remote-apply-ID persistence with a fail-closed guard and emits a new conflict metric. |
| pkg/tern/grpc_client_deployment_id_test.go | Adds focused tests for the new deployment-shared remote apply ID invariant on the gRPC client write path. |
| pkg/storage/deployment_remote_apply.go | Adds helpers to resolve per-operation and per-deployment remote apply IDs (with legacy fallback) and fail closed on disagreement. |
| pkg/storage/deployment_remote_apply_test.go | Adds unit tests covering RemoteApplyID() and DeploymentRemoteApplyID() behavior, including disagreement cases. |
| pkg/metrics/README.md | Documents the new conflict counter and the expected operator response. |
| pkg/metrics/metrics.go | Adds RecordRemoteApplyDeploymentIDConflict for emitting the new counter with deployment dimension. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The refusal that protects an operation's already-recorded remote apply id is the same fail-closed divergence the deployment guard counts, so it now emits the conflict counter and an error log carrying the recorded and refused ids for operator correlation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why this matters
Deployment-keyed dispatch attaches every sibling operation into the deployment's one data-plane apply, so all of a deployment's operation rows must record the same remote apply id — that shared id is what an operator greps in the data plane's storage and logs during an incident. Nothing enforced this: a dispatch result carrying a different id (an in-flight apply spanning a dispatch-key rollout, or a data plane that lost its keyed apply and minted a fresh one) would be persisted silently, splitting one deployment across two remote applies and breaking every correlation built on top.
What it does
persistRemoteApplyIDfails closed when storing a dispatch's remote apply id would correlate the operation's deployment to a second remote apply — either because the deployment's siblings already recorded a different id, or because the siblings themselves already disagree. Sibling deployments of the same apply are exempt: they own their own remote applies.storage.DeploymentRemoteApplyIDresolves the single remote apply id shared by a deployment's operations (with the legacy engine-resume-context carrier honored), erroring on disagreement instead of picking one. The write-path guard consumes it now; the status/progress read model consumes it next.ApplyOperation.RemoteApplyIDnames the external-id-else-legacy-carrier lookup both sites used.schemabot.remote_apply_deployment_id_conflict_total(database, environment, deployment) counts the refusals, documented in the metrics README with the operator action.How it moves us toward the northstar
One data-plane apply per deployment; operations dispatch into it.
Opened by Claude (Fable 5).