mirror of
https://github.com/firstsun-dev/git-files-sync.git
synced 2026-07-22 06:54:27 +00:00
fix(services): omit blank sha so creating a new file doesn't 422
Pushing a new file via the single-file push button failed with GitHub HTTP 422. A 404 lookup returns sha === '' for new files, and the manual push path (sync-manager pushFile/conflict resolution) forwarded that empty string into the Contents API request body as "sha":"", which GitHub rejects. Batch push avoided this via `remote.sha || undefined`. Fix at the service layer so every caller is safe: only include sha in the GitHub request body when it is non-empty. Apply the same guard to the GitLab service's last_commit_id for symmetry. Add regression tests for blank-sha pushes on both services. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DwioG4CNKUBuKiZdowLFWe
This commit is contained in:
parent
bcc5cda69c
commit
339d5cbe77
4 changed files with 31 additions and 4 deletions
|
|
@ -39,12 +39,15 @@ export class GitHubService extends BaseGitService implements GitServiceInterface
|
|||
|
||||
async pushFile(path: string, content: string | ArrayBuffer, branch: string, message: string, sha?: string): Promise<{ path: string, sha?: string }> {
|
||||
const url = this.getApiUrl(path);
|
||||
const body = {
|
||||
const body: { message: string; content: string; branch: string; sha?: string } = {
|
||||
message,
|
||||
content: this.encodeContent(content),
|
||||
branch,
|
||||
sha
|
||||
};
|
||||
// GitHub's Contents API rejects a blank sha with HTTP 422. Only include
|
||||
// it when updating an existing file; a 404 lookup yields sha === '' for
|
||||
// new files, which must be created without a sha.
|
||||
if (sha) body.sha = sha;
|
||||
|
||||
const response = await this.safeRequest(url, 'PUT', body);
|
||||
const data = this.parseJson<{ content: { path: string, sha: string } }>(response);
|
||||
|
|
|
|||
|
|
@ -40,13 +40,14 @@ export class GitLabService extends BaseGitService implements GitServiceInterface
|
|||
|
||||
async pushFile(path: string, content: string | ArrayBuffer, branch: string, message: string, sha?: string): Promise<{ path: string, sha?: string }> {
|
||||
const url = this.getApiUrl(path);
|
||||
const body = {
|
||||
const body: { branch: string; content: string; encoding: string; commit_message: string; last_commit_id?: string } = {
|
||||
branch,
|
||||
content: this.encodeContent(content),
|
||||
encoding: 'base64',
|
||||
commit_message: message,
|
||||
last_commit_id: sha
|
||||
};
|
||||
// A blank sha means the file is new: create it (POST) without last_commit_id.
|
||||
if (sha) body.last_commit_id = sha;
|
||||
|
||||
const method = sha ? 'PUT' : 'POST';
|
||||
const response = await this.safeRequest(url, method, body);
|
||||
|
|
|
|||
|
|
@ -70,6 +70,21 @@ describe('GitHubService', () => {
|
|||
expect(call.body).not.toContain('"sha":');
|
||||
});
|
||||
|
||||
it('should omit blank sha so creating a new file does not 422', async () => {
|
||||
// A 404 lookup yields sha === '' for new files; an empty sha sent to
|
||||
// GitHub causes HTTP 422, so it must be dropped from the request body.
|
||||
vi.mocked(requestUrl).mockResolvedValueOnce({
|
||||
status: 201,
|
||||
json: { content: { path: 'new.md', sha: 'new-sha' } }
|
||||
} as unknown as RequestUrlResponse);
|
||||
|
||||
const result = await service.pushFile('new.md', 'content', 'main', 'create', '');
|
||||
|
||||
expect(result).toEqual({ path: 'new.md', sha: 'new-sha' });
|
||||
const call = getLastRequestCall();
|
||||
expect(call.body).not.toContain('"sha":');
|
||||
});
|
||||
|
||||
it('should update existing file correctly (sha provided)', async () => {
|
||||
mockRequest({ status: 200, json: { content: { path: 'existing.md', sha: 'updated-sha' } } });
|
||||
|
||||
|
|
|
|||
|
|
@ -50,6 +50,14 @@ describe('GitLabService', () => {
|
|||
expect(call.body).toContain(btoa('new content'));
|
||||
});
|
||||
|
||||
it('should treat a blank sha as a new file (POST, no last_commit_id)', async () => {
|
||||
mockRequest({ status: 201, json: { file_path: 'test.md' } });
|
||||
await service.pushFile('test.md', 'new content', 'main', 'initial commit', '');
|
||||
const call = getLastRequestCall();
|
||||
expect(call.method).toBe('POST');
|
||||
expect(call.body).not.toContain('last_commit_id');
|
||||
});
|
||||
|
||||
it('should push file content correctly (PUT for existing file)', async () => {
|
||||
mockRequest({ status: 200, json: { file_path: 'test.md' } });
|
||||
const result = await service.pushFile('test.md', 'updated content', 'main', 'update', 'old-sha');
|
||||
|
|
|
|||
Loading…
Reference in a new issue