Conversation
e9ea7d3 to
27e3edd
Compare
27e3edd to
25aedf0
Compare
Confidence Score: 5/5 - Safe to MergeSafe to merge — this release PR for version 0.2.0 appears clean with no issues identified across the reviewed files. The automated review found no logic bugs, security concerns, or correctness problems in any of the 6 reviewed changed files. This looks like a straightforward version bump or release packaging PR with no substantive technical concerns raised. Key Findings:
|
25aedf0 to
03c6c45
Compare
|
🧪 Testing To try out this version of the SDK: Expires at: Fri, 08 May 2026 03:32:12 GMT |
🐤 Canary SummaryThis PR enhances CLI error messaging for base URL configuration:
Affected User Flows
|
Confidence Score: 5/5 - Safe to MergeSafe to merge — this PR cleanly delivers the 0.2.0 release with well-scoped additions including Key Findings:
Files requiring special attention
|
🐤 Canary Proposed TestsNo testable user journeys found for this PR. |
Confidence Score: 4/5 - Mostly SafeSafe to merge — this PR introduces well-scoped features including Key Findings:
Files requiring special attention
|
| @@ -27,6 +29,8 @@ func TestInnerFlagSet(t *testing.T) { | |||
|
|
|||
| for _, tt := range tests { | |||
| t.Run(tt.name, func(t *testing.T) { | |||
There was a problem hiding this comment.
Correctness: Adding t.Parallel() inside the range loop without capturing tt (via tt := tt) causes all parallel subtests to share the same loop variable — in Go < 1.22, by the time the subtests run, tt will hold the last iteration's value, making all tests use identical inputs.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In file `internal/requestflag/innerflag_test.go`, inside the `for _, tt := range tests` loop in `TestInnerFlagSet`, `t.Parallel()` is called without first capturing the loop variable. For Go versions before 1.22, this causes a data race where all parallel subtests may use the last value of `tt`. Add `tt := tt` immediately before `t.Parallel()` on the line after `t.Run(tt.name, func(t *testing.T) {` to create a per-iteration local copy.
| // Test initialization and setting | ||
| t.Run("PreParse initialization", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.NoError(t, strFlag.PreParse()) | ||
| assert.True(t, strFlag.applied) | ||
| assert.Equal(t, "default-string", strFlag.Get()) | ||
| }) | ||
|
|
||
| t.Run("Set string flag", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.NoError(t, strFlag.Set("string-flag", "new-value")) | ||
| assert.Equal(t, "new-value", strFlag.Get()) | ||
| assert.True(t, strFlag.IsSet()) | ||
| }) | ||
|
|
||
| t.Run("Set int flag with valid value", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.NoError(t, superstitiousIntFlag.Set("int-flag", "100")) | ||
| assert.Equal(t, int64(100), superstitiousIntFlag.Get()) | ||
| assert.True(t, superstitiousIntFlag.IsSet()) | ||
| }) | ||
|
|
||
| t.Run("Set int flag with invalid value", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.Error(t, superstitiousIntFlag.Set("int-flag", "not-an-int")) | ||
| }) | ||
|
|
||
| t.Run("Set int flag with validator failing", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.Error(t, superstitiousIntFlag.Set("int-flag", "13")) | ||
| }) | ||
|
|
||
| t.Run("Set bool flag", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.NoError(t, boolFlag.Set("bool-flag", "true")) | ||
| assert.Equal(t, true, boolFlag.Get()) | ||
| assert.True(t, boolFlag.IsSet()) | ||
| }) | ||
|
|
||
| t.Run("Set slice flag with multiple values", func(t *testing.T) { |
There was a problem hiding this comment.
Correctness: Adding t.Parallel() to these subtests while they all share the same strFlag, superstitiousIntFlag, and boolFlag instances (defined in the outer TestFlagSet scope) introduces data races — concurrent calls to PreParse, Set, and Get on the same flag objects will race under -race, causing flaky or incorrect test results.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In `internal/requestflag/requestflag_test.go`, the diff adds `t.Parallel()` to subtests inside `TestFlagSet` (around lines 347–393). However, those subtests all operate on the same shared `strFlag`, `superstitiousIntFlag`, and `boolFlag` variables declared in the outer test function. Running them in parallel causes concurrent reads and writes to those shared `Flag` structs, which is a data race.
Fix: Either (a) remove `t.Parallel()` from the subtests that share these outer-scope flag variables, or (b) move the flag construction inside each subtest so each parallel subtest has its own independent instance. The subtests at the bottom of `TestFlagSet` that already create local `sliceFlag` variables are safe to keep parallel.
Confidence Score: 3/5 - Review RecommendedNot safe to merge without fixes — while this PR delivers meaningful improvements like Key Findings:
Files requiring special attention
|
03c6c45 to
5683b46
Compare
5683b46 to
373f189
Compare
| // Test initialization and setting | ||
| t.Run("PreParse initialization", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.NoError(t, strFlag.PreParse()) | ||
| assert.True(t, strFlag.applied) | ||
| assert.Equal(t, "default-string", strFlag.Get()) | ||
| }) | ||
|
|
||
| t.Run("Set string flag", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.NoError(t, strFlag.Set("string-flag", "new-value")) | ||
| assert.Equal(t, "new-value", strFlag.Get()) | ||
| assert.True(t, strFlag.IsSet()) | ||
| }) | ||
|
|
||
| t.Run("Set int flag with valid value", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.NoError(t, superstitiousIntFlag.Set("int-flag", "100")) | ||
| assert.Equal(t, int64(100), superstitiousIntFlag.Get()) | ||
| assert.True(t, superstitiousIntFlag.IsSet()) | ||
| }) | ||
|
|
||
| t.Run("Set int flag with invalid value", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.Error(t, superstitiousIntFlag.Set("int-flag", "not-an-int")) | ||
| }) | ||
|
|
||
| t.Run("Set int flag with validator failing", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.Error(t, superstitiousIntFlag.Set("int-flag", "13")) | ||
| }) | ||
|
|
||
| t.Run("Set bool flag", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.NoError(t, boolFlag.Set("bool-flag", "true")) | ||
| assert.Equal(t, true, boolFlag.Get()) | ||
| assert.True(t, boolFlag.IsSet()) | ||
| }) | ||
|
|
||
| t.Run("Set slice flag with multiple values", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| sliceFlag := &Flag[[]int64]{ | ||
| Name: "slice-flag", | ||
| Default: []int64{}, |
There was a problem hiding this comment.
Correctness: The subtests now run in parallel but all mutate shared strFlag, superstitiousIntFlag, and boolFlag declared in the outer TestFlagSet scope — this introduces data races on Flag internal fields (value, hasBeenSet, applied, count) with no synchronization, causing non-deterministic test failures and potential panics.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In file `internal/requestflag/requestflag_test.go`, the `t.Parallel()` calls added to subtests inside `TestFlagSet` (starting around line 348) cause data races because `strFlag`, `superstitiousIntFlag`, and `boolFlag` are shared mutable state across parallel subtests. Fix this by either: (1) removing `t.Parallel()` from all subtests that share these outer-scope flag variables, or (2) moving the flag declarations inside each subtest so each parallel subtest operates on its own independent flag instance.
pkg/cmd/memory.go
Outdated
| Usage: "The file to ingest.", | ||
| Required: true, | ||
| BodyPath: "file", | ||
| }, | ||
| &requestflag.Flag[any]{ |
There was a problem hiding this comment.
Correctness: Removing FileInput: true means the requestflag package will no longer treat this flag as a file path to read and stream — it will pass the raw string value (the filename) as the body instead of the file contents, silently breaking the upload endpoint.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In pkg/cmd/memory.go around line 375, the `FileInput: true` field was removed from the `requestflag.Flag[string]` struct for the 'file' flag in the `memoriesUpload` command. This field is required for the requestflag package to read the actual file contents from the provided path and pass them to the multipart form upload. Without it, only the raw filename string is sent as the body, breaking file uploads silently. Restore `FileInput: true` to the flag definition.
| @@ -114,6 +116,8 @@ func TestEncode(t *testing.T) { | |||
|
|
|||
| for name, test := range tests { | |||
| t.Run(name, func(t *testing.T) { | |||
There was a problem hiding this comment.
Correctness: Adding t.Parallel() inside the range loop without capturing name and test per iteration causes all subtests to close over the same loop variables — in Go < 1.22 they will all run with the last iteration's values, producing incorrect/flaky test results.
Affected Locations:
- internal/apiquery/query_test.go:118-118
- internal/requestflag/innerflag_test.go:31-31
- internal/requestflag/requestflag_test.go:60-61
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In internal/apiquery/query_test.go, around line 116-117, the for-range loop over `tests` was given a `t.Parallel()` call inside the `t.Run` callback. Without capturing the loop variables locally, all parallel subtests will share the same `name` and `test` variables (the last iteration's values in Go < 1.22), causing incorrect test behaviour. Add `name, test := name, test` immediately after the `t.Run` open brace (before `t.Parallel()`) to shadow and capture each iteration's values.
Confidence Score: 1/5 - Blocking IssuesNot safe to merge — this PR introduces multiple correctness-breaking bugs that will cause non-deterministic test failures and a silent functional regression in production. In Key Findings:
Files requiring special attention
|
373f189 to
b0b376a
Compare
b0b376a to
4d95a0b
Compare
| // Test initialization and setting | ||
| t.Run("PreParse initialization", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.NoError(t, strFlag.PreParse()) | ||
| assert.True(t, strFlag.applied) | ||
| assert.Equal(t, "default-string", strFlag.Get()) | ||
| }) | ||
|
|
||
| t.Run("Set string flag", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.NoError(t, strFlag.Set("string-flag", "new-value")) | ||
| assert.Equal(t, "new-value", strFlag.Get()) | ||
| assert.True(t, strFlag.IsSet()) | ||
| }) | ||
|
|
||
| t.Run("Set int flag with valid value", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.NoError(t, superstitiousIntFlag.Set("int-flag", "100")) | ||
| assert.Equal(t, int64(100), superstitiousIntFlag.Get()) | ||
| assert.True(t, superstitiousIntFlag.IsSet()) | ||
| }) | ||
|
|
||
| t.Run("Set int flag with invalid value", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.Error(t, superstitiousIntFlag.Set("int-flag", "not-an-int")) | ||
| }) | ||
|
|
||
| t.Run("Set int flag with validator failing", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.Error(t, superstitiousIntFlag.Set("int-flag", "13")) | ||
| }) | ||
|
|
||
| t.Run("Set bool flag", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.NoError(t, boolFlag.Set("bool-flag", "true")) | ||
| assert.Equal(t, true, boolFlag.Get()) | ||
| assert.True(t, boolFlag.IsSet()) | ||
| }) | ||
|
|
There was a problem hiding this comment.
Correctness: All subtests now run in parallel but share the same strFlag, superstitiousIntFlag, and boolFlag instances defined in the outer scope — concurrent reads and writes to their internal mutable fields (value, hasBeenSet, applied, count) will cause data races detected by go test -race and produce non-deterministic results.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In `internal/requestflag/requestflag_test.go`, the subtests inside `TestFlagSet` (starting around line 347) were made parallel via `t.Parallel()`, but they all share the same `strFlag`, `superstitiousIntFlag`, and `boolFlag` variables declared in the outer function scope. These `Flag` structs have mutable fields (`value`, `hasBeenSet`, `applied`, `count`) that are written concurrently, causing data races. Fix this by either: (1) removing `t.Parallel()` from subtests that share these outer variables, or (2) moving the flag construction inside each subtest so each parallel subtest has its own local instance.
| Suggest: true, | ||
| Flags: []cli.Flag{ | ||
| &requestflag.Flag[string]{ | ||
| Name: "file", | ||
| Usage: "The file to ingest.", | ||
| Required: true, | ||
| BodyPath: "file", | ||
| FileInput: true, | ||
| Name: "file", | ||
| Usage: "The file to ingest.", | ||
| Required: true, | ||
| BodyPath: "file", | ||
| }, | ||
| &requestflag.Flag[any]{ | ||
| Name: "collection", |
There was a problem hiding this comment.
Correctness: The FileInput: true field has been removed from the file flag, which likely means the CLI will pass the raw string (file path) instead of reading and streaming the file contents for multipart upload — breaking the memories upload command.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In pkg/cmd/memory.go around line 371-380, the `FileInput: true` field was removed from the `requestflag.Flag[string]` definition for the 'file' flag in the `memoriesUpload` command. This field is required so the CLI reads the file from disk and passes its contents (not just the path string) to the multipart form upload. Restore `FileInput: true` to the flag definition to fix the upload functionality.
Confidence Score: 2/5 - Changes NeededNot safe to merge — this PR introduces three high-severity bugs that must be addressed before merging. In Key Findings:
Files requiring special attention
|
| @@ -114,6 +116,8 @@ func TestEncode(t *testing.T) { | |||
|
|
|||
| for name, test := range tests { | |||
| t.Run(name, func(t *testing.T) { | |||
There was a problem hiding this comment.
Correctness: Adding t.Parallel() inside the subtest causes the closure to capture the test loop variable by reference; in Go < 1.22 all parallel subtests will race on the same test value, producing incorrect or flaky results. A local copy (test := test) is needed before t.Parallel() to pin the value per iteration.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In internal/apiquery/query_test.go, around line 118, the subtest closure added `t.Parallel()` but the `test` loop variable is captured by reference. In Go versions before 1.22, all parallel subtests will share the same loop variable, causing data races and incorrect test behavior. Fix by adding `test := test` immediately before `t.Parallel()` inside the closure to create a per-iteration copy of the loop variable.
| // Test initialization and setting | ||
| t.Run("PreParse initialization", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.NoError(t, strFlag.PreParse()) | ||
| assert.True(t, strFlag.applied) | ||
| assert.Equal(t, "default-string", strFlag.Get()) | ||
| }) | ||
|
|
||
| t.Run("Set string flag", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.NoError(t, strFlag.Set("string-flag", "new-value")) | ||
| assert.Equal(t, "new-value", strFlag.Get()) | ||
| assert.True(t, strFlag.IsSet()) | ||
| }) | ||
|
|
||
| t.Run("Set int flag with valid value", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.NoError(t, superstitiousIntFlag.Set("int-flag", "100")) | ||
| assert.Equal(t, int64(100), superstitiousIntFlag.Get()) | ||
| assert.True(t, superstitiousIntFlag.IsSet()) | ||
| }) | ||
|
|
||
| t.Run("Set int flag with invalid value", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.Error(t, superstitiousIntFlag.Set("int-flag", "not-an-int")) | ||
| }) | ||
|
|
||
| t.Run("Set int flag with validator failing", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.Error(t, superstitiousIntFlag.Set("int-flag", "13")) | ||
| }) | ||
|
|
||
| t.Run("Set bool flag", func(t *testing.T) { | ||
| t.Parallel() |
There was a problem hiding this comment.
Correctness: The subtests PreParse initialization, Set string flag, Set int flag with valid value, Set int flag with invalid value, Set int flag with validator failing, and Set bool flag all share the same strFlag, superstitiousIntFlag, and boolFlag pointers declared in the outer TestFlagSet scope. Adding t.Parallel() to these subtests causes concurrent reads and writes to those shared flag structs (mutating value, hasBeenSet, applied, count), introducing data races that will produce non-deterministic failures or corrupt state under -race.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In `internal/requestflag/requestflag_test.go`, the subtests inside `TestFlagSet` that were made parallel (lines ~348-386) all mutate shared flag instances (`strFlag`, `superstitiousIntFlag`, `boolFlag`) declared in the outer function scope. This creates data races. Fix this by either: (1) removing `t.Parallel()` from all subtests that share these outer-scope flag variables, or (2) constructing a fresh flag instance inside each subtest instead of sharing the outer-scope ones. The 'Set slice flag' subtests are fine since they already create local flags.
| Suggest: true, | ||
| Flags: []cli.Flag{ | ||
| &requestflag.Flag[string]{ | ||
| Name: "file", | ||
| Usage: "The file to ingest.", | ||
| Required: true, | ||
| BodyPath: "file", | ||
| FileInput: true, | ||
| Name: "file", | ||
| Usage: "The file to ingest.", | ||
| Required: true, | ||
| BodyPath: "file", | ||
| }, | ||
| &requestflag.Flag[any]{ | ||
| Name: "collection", |
There was a problem hiding this comment.
Correctness: Removing FileInput: true means the file flag will no longer be treated as a file path to open and read — it will be passed as a raw string, causing multipart uploads to send the filename string instead of the actual file contents.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In pkg/cmd/memory.go around line 371-380, the `FileInput: true` field was removed from the `requestflag.Flag[string]` definition for the 'file' flag in the `memoriesUpload` command. This field is responsible for instructing the flag processing logic to open the file at the given path and read its contents for multipart upload. Without it, the flag value is treated as a plain string (the filename), not the actual file data, breaking file uploads. Please restore `FileInput: true` to this flag definition.
Confidence Score: 1/5 - Blocking IssuesNot safe to merge — this PR introduces data races in parallelized test suites and a functional regression in multipart file uploads. In Key Findings:
Files requiring special attention
|
4d95a0b to
06e8fbe
Compare
| // Test initialization and setting | ||
| t.Run("PreParse initialization", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.NoError(t, strFlag.PreParse()) | ||
| assert.True(t, strFlag.applied) | ||
| assert.Equal(t, "default-string", strFlag.Get()) | ||
| }) | ||
|
|
||
| t.Run("Set string flag", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.NoError(t, strFlag.Set("string-flag", "new-value")) | ||
| assert.Equal(t, "new-value", strFlag.Get()) | ||
| assert.True(t, strFlag.IsSet()) | ||
| }) | ||
|
|
||
| t.Run("Set int flag with valid value", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.NoError(t, superstitiousIntFlag.Set("int-flag", "100")) | ||
| assert.Equal(t, int64(100), superstitiousIntFlag.Get()) | ||
| assert.True(t, superstitiousIntFlag.IsSet()) | ||
| }) | ||
|
|
||
| t.Run("Set int flag with invalid value", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.Error(t, superstitiousIntFlag.Set("int-flag", "not-an-int")) | ||
| }) | ||
|
|
||
| t.Run("Set int flag with validator failing", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.Error(t, superstitiousIntFlag.Set("int-flag", "13")) | ||
| }) | ||
|
|
||
| t.Run("Set bool flag", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.NoError(t, boolFlag.Set("bool-flag", "true")) | ||
| assert.Equal(t, true, boolFlag.Get()) | ||
| assert.True(t, boolFlag.IsSet()) | ||
| }) | ||
|
|
||
| t.Run("Set slice flag with multiple values", func(t *testing.T) { |
There was a problem hiding this comment.
Correctness: Adding t.Parallel() to subtests that all mutate the same outer-scope strFlag, superstitiousIntFlag, and boolFlag objects introduces data races - e.g. 'PreParse initialization' and 'Set string flag' both access/mutate strFlag concurrently, and the three int-flag subtests all race on superstitiousIntFlag and its count field. This will cause non-deterministic test failures and data races detectable by -race.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In internal/requestflag/requestflag_test.go, the subtests inside TestFlagSet that were made parallel in this diff all share the outer-scope variables `strFlag`, `superstitiousIntFlag`, and `boolFlag`. Running these subtests in parallel causes data races because multiple goroutines read and write to the same Flag objects (including the `count`, `value`, `hasBeenSet`, and `applied` fields). Fix this by either: (a) removing `t.Parallel()` from the subtests that share these outer variables, or (b) making each subtest that needs parallelism create its own local flag instance rather than using the shared outer-scope ones.
| // Test initialization and setting | ||
| t.Run("PreParse initialization", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.NoError(t, strFlag.PreParse()) | ||
| assert.True(t, strFlag.applied) | ||
| assert.Equal(t, "default-string", strFlag.Get()) | ||
| }) | ||
|
|
||
| t.Run("Set string flag", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.NoError(t, strFlag.Set("string-flag", "new-value")) | ||
| assert.Equal(t, "new-value", strFlag.Get()) | ||
| assert.True(t, strFlag.IsSet()) | ||
| }) | ||
|
|
||
| t.Run("Set int flag with valid value", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.NoError(t, superstitiousIntFlag.Set("int-flag", "100")) | ||
| assert.Equal(t, int64(100), superstitiousIntFlag.Get()) | ||
| assert.True(t, superstitiousIntFlag.IsSet()) | ||
| }) | ||
|
|
||
| t.Run("Set int flag with invalid value", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.Error(t, superstitiousIntFlag.Set("int-flag", "not-an-int")) | ||
| }) | ||
|
|
||
| t.Run("Set int flag with validator failing", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.Error(t, superstitiousIntFlag.Set("int-flag", "13")) | ||
| }) | ||
|
|
||
| t.Run("Set bool flag", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.NoError(t, boolFlag.Set("bool-flag", "true")) | ||
| assert.Equal(t, true, boolFlag.Get()) | ||
| assert.True(t, boolFlag.IsSet()) | ||
| }) | ||
|
|
||
| t.Run("Set slice flag with multiple values", func(t *testing.T) { |
There was a problem hiding this comment.
Correctness: Adding t.Parallel() to subtests that all mutate the same outer-scope strFlag, superstitiousIntFlag, and boolFlag objects introduces data races - e.g. 'PreParse initialization' and 'Set string flag' both access/mutate strFlag concurrently, and the three int-flag subtests all race on superstitiousIntFlag and its count field. This will cause non-deterministic test failures and data races detectable by -race.
Affected Locations:
- internal/requestflag/requestflag_test.go:348-393
- internal/apiform/form_test.go:90-92
- internal/apiquery/query_test.go:118-119
- internal/requestflag/innerflag_test.go:29-31
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In internal/requestflag/requestflag_test.go, the subtests inside TestFlagSet that were made parallel in this diff all share the outer-scope variables `strFlag`, `superstitiousIntFlag`, and `boolFlag`. Running these subtests in parallel causes data races because multiple goroutines read and write to the same Flag objects (including the `count`, `value`, `hasBeenSet`, and `applied` fields). Fix this by either: (a) removing `t.Parallel()` from the subtests that share these outer variables, or (b) making each subtest that needs parallelism create its own local flag instance rather than using the shared outer-scope ones.
Confidence Score: 2/5 - Changes NeededNot safe to merge — the PR introduces data races in Key Findings:
Files requiring special attention
|
Automated Release PR
0.2.0 (2026-04-08)
Full Changelog: v0.1.0...v0.2.0
Features
-as value representing stdin to binary-only file parameters in CLIs (40329ac)*_BASE_URL/--base-url(665e431)Bug Fixes
Chores
t.Parallel()(5fc2402)os.Stdoutisn't necessary (b7fd553)os.Chdirtot.Chdir(2f49e45)This pull request is managed by Stainless's GitHub App.
The semver version number is based on included commit messages. Alternatively, you can manually set the version number in the title of this pull request.
For a better experience, it is recommended to use either rebase-merge or squash-merge when merging this pull request.
🔗 Stainless website
📚 Read the docs
🙋 Reach out for help or questions