[wanlin31]

No description provided.

[gemini-code-assist]

Code Review

This pull request introduces a validation script (scripts/validate.sh) and updates contract tests to align with CLI flag changes (such as renaming --trigger-id to --id for triggers) and to handle new error messages. Feedback on the changes includes correcting a non-existent staticcheck version (v0.7.0 to v0.5.1) in the validation script, and improving the precision of test assertions in required_flags_test.go and files_upload_test.go by checking for specific error messages rather than using loose combined checks.

[gemini-code-assist]

The specified version of staticcheck (v0.7.0) does not exist. The latest stable version of staticcheck is v0.5.1. Using a non-existent version will cause the validation script to fail during execution. Please use a valid, existing version.

Suggested change

	go run honnef.co/go/tools/cmd/staticcheck@v0.7.0 \
	-checks=inherit,-SA1019,-SA5008,-S1040,-U1000 ./...
	go run honnef.co/go/tools/cmd/staticcheck@v0.5.1 -checks=inherit,-SA1019,-SA5008,-S1040,-U1000 ./...

[gemini-code-assist]

The test assertions for absent vs empty/blank flags are combined using a loose 'neither' check. This reduces the precision of the test, as an absent flag could incorrectly pass if it triggers the path parameter validation instead of Cobra's required flag validation. We should assert the specific expected error message based on whether the flag is absent or empty/blank.

                    var wantErr string
                    if label == "absent" {
                        wantErr = "missing required flag: " + tc.flag
                    } else {
                        wantErr = "blank; path parameters require a non-empty value"
                    }
                    if !strings.Contains(result.stderr, wantErr) {
                        t.Errorf("%v stderr missing expected error %q:\n%s", mode, wantErr, result.stderr)
                    }

[gemini-code-assist]

The test assertions for invalid API versions are combined using a loose check. We should assert the specific expected error message based on whether the version is empty or contains a dot segment to ensure precise validation behavior.

                var wantErr string
                if version == "" {
                    wantErr = "API version must be non-empty"
                } else {
                    wantErr = "contains a dot segment"
                }
                if result.err == nil || strings.Contains(result.stderr, "[DRY-RUN]") || !strings.Contains(result.stderr, wantErr) {
                    t.Errorf("--api-version %q %v was accepted or did not fail with %q:\n%s", version, mode, wantErr, result.stderr)
                }

[wanlin31]

This is okay because we added

for name, value := range parsedParameters {
    for _, segment := range strings.Split(value, "/") {
        if segment == "." || segment == ".." {
            return "", fmt.Errorf("path parameter %q contains a dot segment", name)
        }
    }
}

in internal/sdk/sdkinternal/utils/pathparams.go

[wanlin31]

This test to make sure we reject the unknown flag, but since we have updated to

func newPlannedCmd(name, short, long, groupID, note string) *cobra.Command {
	cmd := &cobra.Command{
		Use:                name,
		Short:              short,
		Args:               cobra.ArbitraryArgs,
		FParseErrWhitelist: cobra.FParseErrWhitelist{UnknownFlags: true}, // <-- ADDED
		RunE: func(cmd *cobra.Command, args []string) error {
			if usage.UsageRequested(cmd) {
				return usage.EmitSchema(cmd, cmd.OutOrStdout())
			}
			return output.WithCLIReason(fmt.Errorf("%s", note), output.ReasonCLIUnavailable)
		},
	}
...

The --server got swallow into tokens, and since tokens accept unknown flag, so it would return a different error message, update the test to use agent as agent doesn't accept --server, so it checks if --server is a known persistent flag on the root command