Skip to content

Commit 729b3f0

Browse files
chubes4homeboy-ci[bot]
chubes4
and
homeboy-ci[bot]
authored
fix: commit remote workspace files atomically (#757)
Co-authored-by: homeboy-ci[bot] <266378653+homeboy-ci[bot]@users.noreply.github.com>
1 parent 8fea762 commit 729b3f0

4 files changed

Lines changed: 424 additions & 52 deletions

File tree

‎inc/Abilities/GitHubAbilities.php‎

Lines changed: 201 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4226,6 +4226,207 @@ public static function createOrUpdateFile( array $input ): array|\WP_Error {
42264226
);
42274227
}
42284228

4229+
/**
4230+
* Commit multiple file writes atomically by creating Git objects and moving one ref.
4231+
*
4232+
* Unlike the Contents API, this publishes all file changes in one commit or leaves
4233+
* the branch ref unchanged when any pre-publication step fails.
4234+
*
4235+
* @param array $input {
4236+
* Required: repo, files, commit_message.
4237+
* Optional: branch.
4238+
* }
4239+
* @return array|\WP_Error Success payload or error.
4240+
*/
4241+
public static function commitFiles( array $input ): array|\WP_Error {
4242+
$repo = self::resolveRepo(sanitize_text_field($input['repo'] ?? ''));
4243+
if ( empty($repo) ) {
4244+
return new \WP_Error('missing_repo', 'Repository (owner/repo) is required or configure a default repo.', array( 'status' => 400 ));
4245+
}
4246+
4247+
$commit_message = sanitize_text_field($input['commit_message'] ?? '');
4248+
if ( empty($commit_message) ) {
4249+
return new \WP_Error('missing_commit_message', 'Commit message is required.', array( 'status' => 400 ));
4250+
}
4251+
4252+
$files = self::normalizeCommitFiles($input['files'] ?? array());
4253+
if ( is_wp_error($files) ) {
4254+
return $files;
4255+
}
4256+
if ( empty($files) ) {
4257+
return new \WP_Error('missing_files', 'At least one file is required for an atomic GitHub commit.', array( 'status' => 400 ));
4258+
}
4259+
4260+
$pat = self::getPatForRepo($repo);
4261+
if ( empty($pat) ) {
4262+
return self::patError();
4263+
}
4264+
4265+
$branch = sanitize_text_field($input['branch'] ?? '');
4266+
if ( '' === $branch ) {
4267+
$branch = self::resolveDefaultBranch($repo, $pat);
4268+
if ( is_wp_error($branch) ) {
4269+
return $branch;
4270+
}
4271+
} else {
4272+
$branch_result = self::ensureBranchExists($repo, $branch, $pat);
4273+
if ( is_wp_error($branch_result) ) {
4274+
return $branch_result;
4275+
}
4276+
}
4277+
4278+
$ref_url = sprintf('%s/repos/%s/git/ref/heads/%s', self::API_BASE, $repo, rawurlencode($branch));
4279+
$ref = self::apiGet($ref_url, array(), $pat);
4280+
if ( is_wp_error($ref) ) {
4281+
return $ref;
4282+
}
4283+
4284+
$base_commit_sha = (string) ( $ref['data']['object']['sha'] ?? '' );
4285+
if ( '' === $base_commit_sha ) {
4286+
return new \WP_Error('github_ref_sha_missing', 'GitHub did not return a SHA for the target branch ref.', array( 'status' => 500 ));
4287+
}
4288+
4289+
$base_commit = self::apiGet(sprintf('%s/repos/%s/git/commits/%s', self::API_BASE, $repo, rawurlencode($base_commit_sha)), array(), $pat);
4290+
if ( is_wp_error($base_commit) ) {
4291+
return $base_commit;
4292+
}
4293+
4294+
$base_tree_sha = (string) ( $base_commit['data']['tree']['sha'] ?? '' );
4295+
if ( '' === $base_tree_sha ) {
4296+
return new \WP_Error('github_base_tree_sha_missing', 'GitHub did not return a tree SHA for the target branch commit.', array( 'status' => 500 ));
4297+
}
4298+
4299+
$tree_entries = array();
4300+
foreach ( $files as $file ) {
4301+
$blob = self::apiRequest(
4302+
'POST',
4303+
sprintf('%s/repos/%s/git/blobs', self::API_BASE, $repo),
4304+
array(
4305+
'content' => (string) $file['content'],
4306+
'encoding' => 'utf-8',
4307+
),
4308+
$pat
4309+
);
4310+
if ( is_wp_error($blob) ) {
4311+
return $blob;
4312+
}
4313+
4314+
$blob_sha = (string) ( $blob['data']['sha'] ?? '' );
4315+
if ( '' === $blob_sha ) {
4316+
return new \WP_Error('github_blob_sha_missing', sprintf('GitHub did not return a blob SHA for %s.', $file['path']), array( 'status' => 500 ));
4317+
}
4318+
4319+
$tree_entries[] = array(
4320+
'path' => $file['path'],
4321+
'mode' => '100644',
4322+
'type' => 'blob',
4323+
'sha' => $blob_sha,
4324+
);
4325+
}
4326+
4327+
$tree = self::apiRequest(
4328+
'POST',
4329+
sprintf('%s/repos/%s/git/trees', self::API_BASE, $repo),
4330+
array(
4331+
'base_tree' => $base_tree_sha,
4332+
'tree' => $tree_entries,
4333+
),
4334+
$pat
4335+
);
4336+
if ( is_wp_error($tree) ) {
4337+
return $tree;
4338+
}
4339+
4340+
$new_tree_sha = (string) ( $tree['data']['sha'] ?? '' );
4341+
if ( '' === $new_tree_sha ) {
4342+
return new \WP_Error('github_tree_sha_missing', 'GitHub did not return a tree SHA for the atomic commit.', array( 'status' => 500 ));
4343+
}
4344+
4345+
$commit = self::apiRequest(
4346+
'POST',
4347+
sprintf('%s/repos/%s/git/commits', self::API_BASE, $repo),
4348+
array(
4349+
'message' => $commit_message,
4350+
'tree' => $new_tree_sha,
4351+
'parents' => array( $base_commit_sha ),
4352+
),
4353+
$pat
4354+
);
4355+
if ( is_wp_error($commit) ) {
4356+
return $commit;
4357+
}
4358+
4359+
$new_commit_sha = (string) ( $commit['data']['sha'] ?? '' );
4360+
if ( '' === $new_commit_sha ) {
4361+
return new \WP_Error('github_commit_sha_missing', 'GitHub did not return a SHA for the atomic commit.', array( 'status' => 500 ));
4362+
}
4363+
4364+
$updated_ref = self::apiRequest(
4365+
'PATCH',
4366+
$ref_url,
4367+
array(
4368+
'sha' => $new_commit_sha,
4369+
'force' => false,
4370+
),
4371+
$pat
4372+
);
4373+
if ( is_wp_error($updated_ref) ) {
4374+
return $updated_ref;
4375+
}
4376+
4377+
return array(
4378+
'success' => true,
4379+
'commit' => array(
4380+
'sha' => $new_commit_sha,
4381+
'html_url' => $commit['data']['html_url'] ?? '',
4382+
'message' => $commit['data']['message'] ?? $commit_message,
4383+
),
4384+
'branch' => $branch,
4385+
'files' => array_map(static fn( array $file ): string => (string) $file['path'], $files),
4386+
'message' => sprintf('Committed %d file(s) to %s in %s.', count($files), $branch, $repo),
4387+
);
4388+
}
4389+
4390+
/**
4391+
* Normalize an atomic commit file set.
4392+
*
4393+
* @param mixed $files Associative path => content map or list of path/content arrays.
4394+
* @return array<int,array{path:string,content:string}>|\WP_Error
4395+
*/
4396+
private static function normalizeCommitFiles( mixed $files ): array|\WP_Error {
4397+
if ( ! is_array($files) ) {
4398+
return new \WP_Error('invalid_files', 'Files must be an array.', array( 'status' => 400 ));
4399+
}
4400+
4401+
$normalized = array();
4402+
foreach ( $files as $key => $value ) {
4403+
if ( is_array($value) ) {
4404+
$path = self::normalizeRepoWritePath($value['path'] ?? $key);
4405+
$content = $value['content'] ?? '';
4406+
} else {
4407+
$path = self::normalizeRepoWritePath($key);
4408+
$content = $value;
4409+
}
4410+
4411+
if ( is_wp_error($path) ) {
4412+
return $path;
4413+
}
4414+
if ( '' === $path ) {
4415+
return new \WP_Error('missing_file_path', 'Each file in an atomic GitHub commit requires a path.', array( 'status' => 400 ));
4416+
}
4417+
if ( ! is_scalar($content) && null !== $content ) {
4418+
return new \WP_Error('invalid_file_content', sprintf('File content for %s must be scalar.', $path), array( 'status' => 400 ));
4419+
}
4420+
4421+
$normalized[ $path ] = array(
4422+
'path' => $path,
4423+
'content' => (string) $content,
4424+
);
4425+
}
4426+
4427+
return array_values($normalized);
4428+
}
4429+
42294430
/**
42304431
* Normalize a repository write path without allowing traversal outside the repo.
42314432
*

‎inc/Workspace/RemoteWorkspaceBackend.php‎

Lines changed: 12 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -877,7 +877,7 @@ public function git_add( string $handle, array $paths ): array|\WP_Error {
877877
}
878878

879879
/**
880-
* Commit pending remote workspace changes through GitHub Contents API.
880+
* Commit pending remote workspace changes through one GitHub Git-data commit.
881881
*
882882
* @return array<string,mixed>|\WP_Error
883883
*/
@@ -899,75 +899,35 @@ public function git_commit( string $handle, string $message ): array|\WP_Error {
899899
return new \WP_Error('nothing_to_commit', 'No remote workspace changes to commit.', array( 'status' => 400 ));
900900
}
901901

902-
$last_sha = '';
903-
foreach ( $pending as $path => $content ) {
904-
$current_sha = $this->file_sha_for_commit($context, (string) $path);
905-
if ( is_wp_error($current_sha) ) {
906-
return $current_sha;
907-
}
908-
909-
$input = array(
902+
$result = GitHubAbilities::commitFiles(
903+
array(
910904
'repo' => $context['repo'],
911-
'file_path' => $path,
912-
'content' => (string) $content,
905+
'files' => $pending,
913906
'commit_message' => $message,
914907
'branch' => $context['branch'],
915-
);
916-
if ( '' !== $current_sha ) {
917-
$input['sha'] = $current_sha;
918-
}
919-
920-
$result = GitHubAbilities::createOrUpdateFile(
921-
$input
922-
);
923-
if ( is_wp_error($result) ) {
924-
return $result;
925-
}
926-
$last_sha = (string) ( $result['commit']['sha'] ?? $last_sha );
908+
)
909+
);
910+
if ( is_wp_error($result) ) {
911+
return $result;
927912
}
928913

914+
$commit_sha = (string) ( $result['commit']['sha'] ?? '' );
915+
929916
$state = $this->state();
930917
$state['worktrees'][ $context['handle'] ]['pending_files'] = array();
931-
$state['worktrees'][ $context['handle'] ]['last_commit_sha'] = $last_sha;
918+
$state['worktrees'][ $context['handle'] ]['last_commit_sha'] = $commit_sha;
932919
$this->save_state($state);
933920

934921
return array(
935922
'success' => true,
936923
'backend' => 'github_api',
937924
'name' => $handle,
938925
'branch' => $context['branch'],
939-
'commit' => $last_sha,
926+
'commit' => $commit_sha,
940927
'message' => sprintf('Committed remote workspace changes to %s.', $context['branch']),
941928
);
942929
}
943930

944-
/**
945-
* Resolve the current file SHA for a Contents API update.
946-
*
947-
* @param array<string,mixed> $context Remote workspace context.
948-
*/
949-
private function file_sha_for_commit( array $context, string $path ): string|\WP_Error {
950-
$file = $this->get_file_contents_with_fallback($context, $path);
951-
if ( is_wp_error($file) ) {
952-
$status = (int) ( $file->get_error_data()['status'] ?? 0 );
953-
if ( 404 === $status ) {
954-
return '';
955-
}
956-
957-
return $file;
958-
}
959-
if ( empty($file['files'][0]) ) {
960-
$status = (int) ( $file['errors'][0]['status'] ?? 0 );
961-
if ( 404 === $status ) {
962-
return '';
963-
}
964-
965-
return new \WP_Error('remote_workspace_file_unavailable', $file['errors'][0]['message'] ?? sprintf('File not found: %s.', $path), array( 'status' => 404 ));
966-
}
967-
968-
return (string) ( $file['files'][0]['sha'] ?? '' );
969-
}
970-
971931
/**
972932
* Read GitHub contents for a worktree path, falling back to the default branch
973933
* when the remote worktree branch has not been materialized yet.

‎tests/TESTING.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ php tests/smoke-worktree-handles.php # pure-unit, fast
1212
php tests/smoke-worktree-cleanup.php # spawns a real git workspace
1313
php tests/smoke-worktree-cleanup-merged-obsolete-dirty.php # merged + obsolete-on-default classifier
1414
php tests/smoke-worktree-bootstrap.php # fixture + real git, no WP required
15+
php tests/github-atomic-commit.php # GitHub API stubs, no network
1516
```
1617

1718
Expected: `32/32 passed`, `131/131 passed`, `14/14 passed`, and `30/30 passed`

0 commit comments

Comments
 (0)