Set up goreleaser and container build#4
Conversation
Add dockerfile and set up goreleaser for making releases.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| - name: Set up Go | ||
| uses: actions/setup-go@v5 | ||
| - name: Run GoReleaser |
There was a problem hiding this comment.
Provide a Go version to
actions/setup-go
actions/setup-go@v5 requires either go-version or go-version-file; without one it immediately fails with Input required and not supplied: go-version. As written, any tag push will stop before GoReleaser runs, so releases cannot be produced until a Go version is declared.
Useful? React with 👍 / 👎.
| ldflags: | ||
| - -s -w | ||
| - -X main.version={{.Version}} | ||
| - -X main.commit={{.Commit}} | ||
| - -X main.date={{.Date}} | ||
| - -X main.builtBy=goreleaser |
There was a problem hiding this comment.
Define variables referenced by GoReleaser ldflags
The GoReleaser build injects -X main.version, -X main.commit, -X main.date, and -X main.builtBy, but the main package does not declare these string variables anywhere. go build fails when an -X target is missing, so tagged releases will error out during compilation. Add these variables to main (e.g. with default values) or drop the ldflags.
Useful? React with 👍 / 👎.
## Summary The merged cluster CLI LLD has a self-inconsistency: §v1 Definition of Done #4 references exit code \`15\` for name collision, but the collapsed exit-code matrix (in §Exit codes) only defines codes \`0/2/3/4/5/10/20/40\` — \`15\` doesn't exist. The correct shape per the matrix design (8 codes total; granular cause lives in \`error.category\`) is exit \`10\` with \`category: "name-collision"\`. Updates the DoD line to match. ## Diff \`\`\`diff - 4. \`seictl bench up --name X\` re-run while the previous bench is still alive: idempotent (no error, returns same result), or rejects with exit \`15\` on name collision — confirmed behavior either way. + 4. \`seictl bench up --name X\` re-run while the previous bench is still alive: idempotent (no error, returns same result), or rejects with exit \`10\` (category \`name-collision\`) — confirmed behavior either way. \`\`\` ## Test plan - [ ] Render the LLD on GitHub and confirm the DoD line reads consistently with the exit-code matrix - [ ] No code impact (this is a docs-only fix) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3-specialist cross-review surfaced 6 must-fix items and 2 should-also-fix items. All applied here. 1. Cancellation produces false 'completed' (k8s B1): pollForInclusion now distinguishes ctx cancellation from deadline. Cancellation returns ctx.Err() so the engine records the task as failed rather than synthesizing a 'completed' record on a truncated run. Next Submit with the same TaskID short-circuits via the persisted checkpoint to the chain-query path. 2. Poll not ctx-aware on network call (platform B1): Check ctx + deadline BEFORE every QueryTx call, not just after. A CLI-driven --timeout 5s now actually bounds a hung-seid wait, independent of the QueryTx transport timeout. 3. Guard ordering (platform B2): Move Checkpoints nil-check BEFORE Keyring.Key() access. A misconfigured checkpoint store no longer triggers an unnecessary passphrase-prompt path on the file backend. 4. Zero-fee accepted (security #1): checkFeesDenom now rejects 0-resolved coins. '0usei' after ParseCoinsNormalized filters to empty Coins; we additionally check !IsAllPositive(). Catches bypasses of the min-gas-price contract. 5. Memo unbounded (security #2): New validateMemo: cap 256 bytes (matches Cosmos MaxMemoCharacters), reject non-printable. A 64KB memo could pollute the result store pre-broadcast; control-char injection could contaminate audit logs. 6. Checkpoint race on same TaskID + different ChainID (security #3): SaveCheckpoint wrapped in BEGIN IMMEDIATE transaction. Pre-check of existing chain_id under the write lock rejects cross-chain TaskID collisions before the INSERT OR REPLACE clobbers anything. 7. CheckTx-rejection now deletes the checkpoint: Stale STALEHASH checkpoint after CheckTx failure was an audit- confusing dangling record. Deleting it leaves the retry path clean: LoadCheckpoint returns nil, fall through to re-sign at unchanged sequence on the next attempt. 8. OpenAPI bearerAuth on all mutating + read endpoints: Previously declared only on POST /v0/tasks. GET (list/get) and DELETE now also require bearerAuth. Code-gen clients see the correct contract regardless of which endpoint they target. Tests: - TestInclusionPollingTimeout_* renamed and split into TestInclusionPollingCtxCancel_ReturnsCtxErr (asserts ctx-cancel surfaces as error) and TestInclusionPollingDeadline_ReturnsNonTerminal (asserts poll-deadline-without-ctx-cancel is non-Terminal success with IncludedAt=nil). Uses var-overridable inclusionPollTimeout for speed instead of injecting a ctx deadline. Deferred (tracked separately, not blocking this PR): - k8s B2: controller#222 needs to inject SEI_KEYRING_BACKEND and SEI_KEYRING_DIR alongside SEI_KEYRING_PASSPHRASE — flagged on #222. - security #4: signTxClientFactory sync guard (parallel-tests race) — follow-up; current t.Cleanup pattern works for sequential. - security #10: polling vs mempool ambiguity (use /unconfirmed_txs to distinguish 'still in mempool' from 'never broadcast') — follow-up. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add dockerfile and set up goreleaser for making releases.