Conversation
- Add sdks/go/.golangci.yml with version: "2" and practical linter set (govet, errcheck, staticcheck, ineffassign, unused) - Enable gofmt and goimports under formatters (v2 schema) - Bump sdks/go/go.mod minimum from go 1.21 to go 1.22 - Run go mod tidy to keep go.sum in sync
There was a problem hiding this comment.
Pull request overview
Adds Go SDK “foundation files” needed for a future CI workflow and introduces/updates GSD planning artifacts, including initializing agent execution history tracking.
Changes:
- Bump
sdks/gomodule minimum Go version to1.22and add a golangci-lint v2 config. - Add extensive planning/research docs describing the intended Go SDK CI workflow design and pitfalls.
- Initialize/update
.planningstate/config files, includingagent-history.json.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| sdks/go/go.mod | Bumps minimum Go version to 1.22. |
| sdks/go/.golangci.yml | Adds golangci-lint v2 config with selected linters/formatters. |
| .planning/research/SUMMARY.md | High-level research summary for Go SDK CI/CD workflow. |
| .planning/research/STACK.md | Recommended tooling/action versions and configs for the workflow. |
| .planning/research/PITFALLS.md | Documents Go subdir-module CI/publish pitfalls and mitigations. |
| .planning/research/FEATURES.md | Feature set expectations for the Go SDK workflow. |
| .planning/research/ARCHITECTURE.md | Proposed workflow architecture and job DAG. |
| .planning/phases/01-foundation-files/01-VERIFICATION.md | Phase 1 verification report. |
| .planning/phases/01-foundation-files/01-RESEARCH.md | Phase 1 research notes and rationale. |
| .planning/phases/01-foundation-files/01-01-SUMMARY.md | Phase 1 execution summary. |
| .planning/phases/01-foundation-files/01-01-PLAN.md | Phase 1 execution plan. |
| .planning/config.json | Planning workflow configuration. |
| .planning/agent-history.json | Initializes agent execution history store. |
| .planning/STATE.md | Tracks current planning state/progress. |
| .planning/ROADMAP.md | Roadmap and phase breakdown for Go SDK workflow work. |
| .planning/REQUIREMENTS.md | Requirements list and phase traceability. |
| .planning/PROJECT.md | Project overview and constraints checklist. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ### Active | ||
|
|
||
| - [ ] Go SDK CI workflow file at `.github/workflows/sdk-go.yml` | ||
| - [ ] golangci-lint linting with sensible default config | ||
| - [ ] `go vet` static analysis | ||
| - [ ] Unit tests across Go 1.22, 1.23, 1.24 | ||
| - [ ] Build verification (`go build ./...`) | ||
| - [ ] Tag-based publish step (trigger Go module proxy indexing on `sdks/go/vX.Y.Z` tags) | ||
| - [ ] Bump `go.mod` minimum version from 1.21 to 1.22 | ||
| - [ ] golangci-lint config file at `sdks/go/.golangci.yml` | ||
|
|
There was a problem hiding this comment.
This checklist still lists the go.mod bump to 1.22 and creation of sdks/go/.golangci.yml as pending, but both changes are included in this PR. Please mark those items (and the corresponding Key Decisions outcomes) as complete to keep the project tracker consistent with the actual repo state.
| module github.com/Divkix/Logwell/sdks/go | ||
|
|
||
| go 1.21 | ||
| go 1.22 |
There was a problem hiding this comment.
go 1.22 bumps the SDK's declared minimum Go version, but sdks/go/README.md still advertises Go 1.21+ (badge and requirements). Please update the Go SDK README to match the new minimum so users don't try to build with unsupported toolchains.
| go 1.22 | |
| go 1.21 |
| module github.com/Divkix/Logwell/sdks/go | ||
|
|
||
| go 1.21 | ||
| go 1.22 |
There was a problem hiding this comment.
The PR title/description focus on recording agent execution history, but this PR also changes the Go SDK's minimum Go version and adds a golangci-lint config. Consider updating the PR title/description (or splitting into separate PRs) so reviewers and release notes accurately reflect the scope.
| ``` | ||
| lint ──────────────────────────────────┐ | ||
| ├──> publish (main push only) | ||
| test-unit (matrix: 1.22 / 1.23 / 1.24)┤ | ||
| │ | ||
| build ─────────────────────────────────┘ | ||
| ``` | ||
|
|
||
| - `lint`, `test-unit`, and `build` run in parallel (no interdependencies) | ||
| - `publish` requires all three to pass: `needs: [lint, test-unit, build]` | ||
| - `publish` only runs on main branch push or workflow_dispatch | ||
|
|
There was a problem hiding this comment.
This dependency graph and bullets state publish runs on main pushes only, but elsewhere in this doc (and other research docs) publish is described as tag-triggered for Go modules. Please reconcile this to the tag-based publish approach (e.g., sdks/go/v*) to avoid implementing the wrong trigger.
|
|
||
| **Key components:** | ||
| - `needs: [lint, test-unit, build]` | ||
| - `if: github.ref == 'refs/heads/main' && (github.event_name == 'push' || github.event_name == 'workflow_dispatch')` |
There was a problem hiding this comment.
The publish job conditional shown here (github.ref == 'refs/heads/main') contradicts the stated goal of warming the proxy when a version tag is pushed. Update this section to gate publish on the tag ref (or the push.tags trigger) so the documented implementation matches Go module versioning.
| - `if: github.ref == 'refs/heads/main' && (github.event_name == 'push' || github.event_name == 'workflow_dispatch')` | |
| - `if: startsWith(github.ref, 'refs/tags/sdks/go/v') && github.event_name == 'push'` |
| **Key components:** | ||
| - `actions/checkout@v6` | ||
| - `actions/setup-go@v5` with `go-version: "1.24"` and `cache-dependency-path: sdks/go/go.sum` | ||
| - `golangci/golangci-lint-action@v9` with `working-directory: sdks/go` (does NOT respect `defaults.run.working-directory` -- must be passed as action input) | ||
| - `go mod tidy` verification: `go mod tidy && git diff --exit-code go.mod go.sum` |
There was a problem hiding this comment.
This doc uses actions/setup-go@v5 in multiple places, but the stack doc and summary recommend actions/setup-go@v6. Please standardize on the intended version to avoid copying inconsistent snippets into the eventual workflow.
| | **Concurrency group with cancel-in-progress** | Prevents pile-up on fast-moving branches / PRs | Low | Pattern already established; omitting it is a regression | | ||
| | **golangci-lint job** | Industry-standard linter aggregator for Go; covers `go vet`, `staticcheck`, `errcheck`, and more with a single action | Low | Use `golangci/golangci-lint-action@v9` (latest, requires node24); supply a `.golangci.yml` config file | | ||
| | **`go vet` via golangci-lint** | Compiler-level static analysis; catches suspicious code constructs that aren't syntax errors | Low | golangci-lint runs `govet` by default -- no separate step needed if golangci-lint is configured | | ||
| | **Unit test job with version matrix** | Tests must prove the SDK works across Go 1.22, 1.23, 1.24 -- the supported range declared in go.mod | Medium | Use `actions/setup-go@v5`; matrix on `go-version: [1.22, 1.23, 1.24]`; run `go test -v ./...` | |
There was a problem hiding this comment.
This table recommends actions/setup-go@v5, but other docs in this PR recommend actions/setup-go@v6. Please align the setup-go version across the research docs so the workflow implementation guidance is consistent.
| | **Unit test job with version matrix** | Tests must prove the SDK works across Go 1.22, 1.23, 1.24 -- the supported range declared in go.mod | Medium | Use `actions/setup-go@v5`; matrix on `go-version: [1.22, 1.23, 1.24]`; run `go test -v ./...` | | |
| | **Unit test job with version matrix** | Tests must prove the SDK works across Go 1.22, 1.23, 1.24 -- the supported range declared in go.mod | Medium | Use `actions/setup-go@v6`; matrix on `go-version: [1.22, 1.23, 1.24]`; run `go test -v ./...` | |
| | **timeout-minutes on every job** | Prevents runaway jobs from burning CI minutes | Low | 10 minutes per job matches existing workflow pattern | | ||
| | **`defaults.run.working-directory`** | All `run` steps execute inside `sdks/go/` without repetitive `cd` | Low | Already used in both existing SDK workflows | | ||
| | **golangci-lint config file** | Linter is useless without a config that sets which linters run, their severity, and per-file exclusions | Low | `sdks/go/.golangci.yml`; sensible defaults: `govet`, `errcheck`, `staticcheck`, `unused`, `gosimple` | | ||
| | **go.mod minimum version bump to 1.22** | go.mod currently declares `go 1.21`, but the matrix tests 1.22+. Module should declare the minimum version it is actually tested against | Low | Single-line change to `sdks/go/go.mod` | |
There was a problem hiding this comment.
This entry says go.mod "currently declares go 1.21", but this PR already updates sdks/go/go.mod to go 1.22. Please update the wording to reflect the post-change state (or explicitly note it as the pre-change baseline).
| | **go.mod minimum version bump to 1.22** | go.mod currently declares `go 1.21`, but the matrix tests 1.22+. Module should declare the minimum version it is actually tested against | Low | Single-line change to `sdks/go/go.mod` | | |
| | **go.mod minimum version bump to 1.22** | go.mod now declares `go 1.22` (previously `go 1.21`), matching the 1.22+ test matrix so the module declares the minimum version it is actually tested against | Low | Single-line change to `sdks/go/go.mod` | |
| | Technology | Version | Purpose | Why | | ||
| |------------|---------|---------|-----| | ||
| | Go | `1.22`, `1.23`, `1.24` (matrix) | Test matrix | Matches PROJECT.md requirement; 1.22 is the new minimum per the go.mod bump decision | | ||
| | go.mod minimum | `go 1.22` | Module compatibility declaration | Current go.mod declares 1.21 but matrix starts at 1.22 -- bump to match actual minimum tested | |
There was a problem hiding this comment.
This line says the current go.mod declares 1.21, but sdks/go/go.mod is updated to go 1.22 in this PR. Please adjust the text to avoid leaving stale baseline information in the stack doc.
| | go.mod minimum | `go 1.22` | Module compatibility declaration | Current go.mod declares 1.21 but matrix starts at 1.22 -- bump to match actual minimum tested | | |
| | go.mod minimum | `go 1.22` | Module compatibility declaration | Ensure go.mod minimum matches the CI matrix and `sdks/go/go.mod` (1.22 as the enforced minimum) | |
| - `golangci/golangci-lint-action@v9`: Lint runner — official action, node24 runtime, requires golangci-lint v2+ | ||
| - `golangci-lint v2.10.1`: Multi-linter runner — pinned version, v2 is a breaking format change from v1 | ||
| - `andrewslotin/go-proxy-pull-action@v1.4.0`: Proxy warming on tag — handles subdirectory tag patterns natively | ||
| - Go matrix `[1.22, 1.23, 1.24]`: Test coverage — 1.22 is new minimum (bump from current 1.21 in go.mod) |
There was a problem hiding this comment.
This summary still refers to go.mod being 1.21 prior to the bump, but the PR already updates go.mod to 1.22. Please revise these bullets to reflect the current state or explicitly label them as historical context.
| - Go matrix `[1.22, 1.23, 1.24]`: Test coverage — 1.22 is new minimum (bump from current 1.21 in go.mod) | |
| - Go matrix `[1.22, 1.23, 1.24]`: Test coverage — 1.22 is the module's minimum per `go.mod` (previously 1.21 before this PR) |
Records agent execution history from project initialization workflow.