Merged
Conversation
Picks up seictl#153 — drops the unused TaskMeta + applyMeta from client.*Task wrappers. Side benefit: PlannedTask.Params.Raw bytes shed the "ID":null suffix that came from json-marshaling the empty embed. No controller code changes — TaskMeta had zero references in this repo (the executor sets TaskRequest.Id directly post-ToTaskRequest since #192). Mid-rollover safe: pre-existing CRD bytes containing "ID":null deserialize cleanly into TaskMeta-less structs (Go's json.Unmarshal silently drops unknown keys). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3 tasks
bdchatham
added a commit
that referenced
this pull request
May 6, 2026
Adds e.params.Validate() to sidecarExecution[T].Execute() before the
ToTaskRequest()/SubmitTask path. Catches malformed task params at
executor-time as a structured terminal failure instead of as an opaque
sidecar HTTP 400.
For most task types Validate() returns nil (empty struct or trivial
validation), so this is a no-op in practice. The two strict paths are:
- sidecar.AssembleAndUploadGenesisTask.Validate() — required-field
+ bech32 checks. Already invoked at planner-time at group.go:45,
so this catch is defense-in-depth.
- configApplyTask.Validate() — delegates to seictl
ConfigApplyTask.Validate(), which calls seiconfig.ValidateIntent.
The planner doesn't currently call this; the executor now does.
Drive-by: drop a stale comment about overriding Id "rather than on
the wrapper's embedded TaskMeta" — TaskMeta no longer exists (removed
in seictl#153, consumed in #195). The comment was rot from #192.
Test fixture fix: TestExecuteGroupPlan_CompletesSuccessfully built
an AssembleAndUploadGenesisTask with only Nodes (missing required
AccountBalance and Namespace). It passed before only because Validate
wasn't invoked; now we populate the required fields, which is the
correct test intent for a happy-path submission test.
Closes the platform-engineer's deferred opportunity flagged in the
cross-review of #192.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2 tasks
bdchatham
added a commit
that referenced
this pull request
May 6, 2026
* refactor(task): invoke Validate() in executor before submit
Adds e.params.Validate() to sidecarExecution[T].Execute() before the
ToTaskRequest()/SubmitTask path. Catches malformed task params at
executor-time as a structured terminal failure instead of as an opaque
sidecar HTTP 400.
For most task types Validate() returns nil (empty struct or trivial
validation), so this is a no-op in practice. The two strict paths are:
- sidecar.AssembleAndUploadGenesisTask.Validate() — required-field
+ bech32 checks. Already invoked at planner-time at group.go:45,
so this catch is defense-in-depth.
- configApplyTask.Validate() — delegates to seictl
ConfigApplyTask.Validate(), which calls seiconfig.ValidateIntent.
The planner doesn't currently call this; the executor now does.
One test fixture fix: TestExecuteGroupPlan_CompletesSuccessfully built
an AssembleAndUploadGenesisTask with only Nodes (missing required
AccountBalance and Namespace). It passed before because Validate
wasn't invoked; now we populate the required fields, which is the
correct test intent for a happy-path submission test.
Closes the platform-engineer's deferred opportunity flagged in the
cross-review of #192.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(task): invoke Validate() in executor before submit
Adds e.params.Validate() to sidecarExecution[T].Execute() before the
ToTaskRequest()/SubmitTask path. Catches malformed task params at
executor-time as a structured terminal failure instead of as an opaque
sidecar HTTP 400.
For most task types Validate() returns nil (empty struct or trivial
validation), so this is a no-op in practice. The two strict paths are:
- sidecar.AssembleAndUploadGenesisTask.Validate() — required-field
+ bech32 checks. Already invoked at planner-time at group.go:45,
so this catch is defense-in-depth.
- configApplyTask.Validate() — delegates to seictl
ConfigApplyTask.Validate(), which calls seiconfig.ValidateIntent.
The planner doesn't currently call this; the executor now does.
Drive-by: drop a stale comment about overriding Id "rather than on
the wrapper's embedded TaskMeta" — TaskMeta no longer exists (removed
in seictl#153, consumed in #195). The comment was rot from #192.
Test fixture fix: TestExecuteGroupPlan_CompletesSuccessfully built
an AssembleAndUploadGenesisTask with only Nodes (missing required
AccountBalance and Namespace). It passed before only because Validate
wasn't invoked; now we populate the required fields, which is the
correct test intent for a happy-path submission test.
Closes the platform-engineer's deferred opportunity flagged in the
cross-review of #192.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <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
1 participant
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.
Summary
Picks up seictl#153 — drops the unused
TaskMeta+applyMetafromclient.*Taskwrappers.v0.0.45 → v0.0.46Side benefit
PlannedTask.Params.Rawbytes shed the"ID":nullsuffix that came from json-marshaling the emptyTaskMetaembed. Cosmetic-only — operators readingkubectl get snd -o yamlsee slightly cleaner persisted task bytes.No controller code changes
TaskMetahad zero references in this repo. Since #192's executor refactor, the controller setsTaskRequest.Iddirectly post-ToTaskRequest(), never touchingTaskMeta.ID. The seictl deletion ripples through cleanly via thego.modbump.Mid-rollover safety
Pre-existing CRD
Params.Rawbytes contain"ID":null(from prior versions of this controller persistingclient.*Taskwith the empty embed). Post-upgrade controllers reading those bytes deserialize them into TaskMeta-less structs — Go'sjson.Unmarshalsilently drops unknown keys. Safe.Verification
GOWORK=off go get github.com/sei-protocol/seictl@v0.0.46— clean 3-line diff (nogo mod tidyworkspace-induced churn)GOWORK=off go build ./...— cleanGOWORK=off go test ./...— all packages pass (node, nodedeployment, noderesource, planner, task)🤖 Generated with Claude Code