Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 61 additions & 2 deletions pkg/github/minimal_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,29 @@ type MinimalIssueComment struct {
UpdatedAt string `json:"updated_at,omitempty"`
}

// MinimalFileContentResponse is the trimmed output type for create/update/delete file responses.
type MinimalFileContentResponse struct {
Content *MinimalFileContent `json:"content,omitempty"`
Commit *MinimalFileCommit `json:"commit,omitempty"`
}

// MinimalFileContent is the trimmed content portion of a file operation response.
type MinimalFileContent struct {
Name string `json:"name"`
Path string `json:"path"`
SHA string `json:"sha"`
Size int `json:"size,omitempty"`
HTMLURL string `json:"html_url"`
}

// MinimalFileCommit is the trimmed commit portion of a file operation response.
type MinimalFileCommit struct {
SHA string `json:"sha"`
Message string `json:"message,omitempty"`
HTMLURL string `json:"html_url,omitempty"`
Author *MinimalCommitAuthor `json:"author,omitempty"`
}

// MinimalPullRequest is the trimmed output type for pull request objects to reduce verbosity.
type MinimalPullRequest struct {
Number int `json:"number"`
Expand Down Expand Up @@ -338,6 +361,42 @@ func convertToMinimalIssueComment(comment *github.IssueComment) MinimalIssueComm
return m
}

func convertToMinimalFileContentResponse(resp *github.RepositoryContentResponse) MinimalFileContentResponse {
m := MinimalFileContentResponse{}

if resp == nil {
return m
}

if c := resp.Content; c != nil {
m.Content = &MinimalFileContent{
Name: c.GetName(),
Path: c.GetPath(),
SHA: c.GetSHA(),
Size: c.GetSize(),
HTMLURL: c.GetHTMLURL(),
}
}

m.Commit = &MinimalFileCommit{
SHA: resp.Commit.GetSHA(),
Message: resp.Commit.GetMessage(),
HTMLURL: resp.Commit.GetHTMLURL(),
}

if author := resp.Commit.Author; author != nil {
m.Commit.Author = &MinimalCommitAuthor{
Name: author.GetName(),
Email: author.GetEmail(),
}
if author.Date != nil {
m.Commit.Author.Date = author.Date.Format(time.RFC3339)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 It's not related to your pr but if we're on it could you replace all usages of 2006-01-02T15:04:05Z with time.RFC3339 please? 🙏 (There are 2 in minimal_types.go)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was going to do it in a separate PR, but don't mind updating this one too

}
}

return m
}

func convertToMinimalPullRequest(pr *github.PullRequest) MinimalPullRequest {
m := MinimalPullRequest{
Number: pr.GetNumber(),
Expand Down Expand Up @@ -480,7 +539,7 @@ func convertToMinimalCommit(commit *github.RepositoryCommit, includeDiffs bool)
Email: commit.Commit.Author.GetEmail(),
}
if commit.Commit.Author.Date != nil {
minimalCommit.Commit.Author.Date = commit.Commit.Author.Date.Format("2006-01-02T15:04:05Z")
minimalCommit.Commit.Author.Date = commit.Commit.Author.Date.Format(time.RFC3339)
}
}

Expand All @@ -490,7 +549,7 @@ func convertToMinimalCommit(commit *github.RepositoryCommit, includeDiffs bool)
Email: commit.Commit.Committer.GetEmail(),
}
if commit.Commit.Committer.Date != nil {
minimalCommit.Commit.Committer.Date = commit.Commit.Committer.Date.Format("2006-01-02T15:04:05Z")
minimalCommit.Commit.Committer.Date = commit.Commit.Committer.Date.Format(time.RFC3339)
}
}
}
Expand Down
13 changes: 7 additions & 6 deletions pkg/github/repositories.go
Original file line number Diff line number Diff line change
Expand Up @@ -489,13 +489,14 @@ If the SHA is not provided, the tool will attempt to acquire it by fetching the
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to create/update file", resp, body), nil, nil
}

r, err := json.Marshal(fileContent)
if err != nil {
return nil, nil, fmt.Errorf("failed to marshal response: %w", err)
}
minimalResponse := convertToMinimalFileContentResponse(fileContent)

// Warn if file was updated without SHA validation (blind update)
if sha == "" && previousSHA != "" {
warning, err := json.Marshal(minimalResponse)
if err != nil {
return nil, nil, fmt.Errorf("failed to marshal response: %w", err)
}
return utils.NewToolResultText(fmt.Sprintf(
"Warning: File updated without SHA validation. Previous file SHA was %s. "+
`Verify no unintended changes were overwritten:
Expand All @@ -504,10 +505,10 @@ If the SHA is not provided, the tool will attempt to acquire it by fetching the
3. Revert changes if shas do not match.

%s`,
previousSHA, path, string(r))), nil, nil
previousSHA, path, string(warning))), nil, nil
}

return utils.NewToolResultText(string(r)), nil, nil
return MarshalledTextResult(minimalResponse), nil, nil
},
)
}
Expand Down
21 changes: 15 additions & 6 deletions pkg/github/repositories_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1434,18 +1434,27 @@ func Test_CreateOrUpdateFile(t *testing.T) {
}

// Unmarshal and verify the result
var returnedContent github.RepositoryContentResponse
var returnedContent MinimalFileContentResponse
err = json.Unmarshal([]byte(textContent.Text), &returnedContent)
require.NoError(t, err)

// Verify content
assert.Equal(t, *tc.expectedContent.Content.Name, *returnedContent.Content.Name)
assert.Equal(t, *tc.expectedContent.Content.Path, *returnedContent.Content.Path)
assert.Equal(t, *tc.expectedContent.Content.SHA, *returnedContent.Content.SHA)
assert.Equal(t, tc.expectedContent.Content.GetName(), returnedContent.Content.Name)
assert.Equal(t, tc.expectedContent.Content.GetPath(), returnedContent.Content.Path)
assert.Equal(t, tc.expectedContent.Content.GetSHA(), returnedContent.Content.SHA)
assert.Equal(t, tc.expectedContent.Content.GetSize(), returnedContent.Content.Size)
assert.Equal(t, tc.expectedContent.Content.GetHTMLURL(), returnedContent.Content.HTMLURL)

// Verify commit
assert.Equal(t, *tc.expectedContent.Commit.SHA, *returnedContent.Commit.SHA)
assert.Equal(t, *tc.expectedContent.Commit.Message, *returnedContent.Commit.Message)
assert.Equal(t, tc.expectedContent.Commit.GetSHA(), returnedContent.Commit.SHA)
assert.Equal(t, tc.expectedContent.Commit.GetMessage(), returnedContent.Commit.Message)
assert.Equal(t, tc.expectedContent.Commit.GetHTMLURL(), returnedContent.Commit.HTMLURL)

// Verify commit author
require.NotNil(t, returnedContent.Commit.Author)
assert.Equal(t, tc.expectedContent.Commit.Author.GetName(), returnedContent.Commit.Author.Name)
assert.Equal(t, tc.expectedContent.Commit.Author.GetEmail(), returnedContent.Commit.Author.Email)
assert.NotEmpty(t, returnedContent.Commit.Author.Date)
})
}
}
Expand Down