Skip to content

Commit b8a4183

Browse files
chubes4chubes4
authored
Fix workspace worktree repo resolution (#639)
* Fix workspace worktree repo resolution * Bound cleanup worktree removal
1 parent 4057301 commit b8a4183

10 files changed

Lines changed: 415 additions & 23 deletions

‎inc/Abilities/WorkspaceAbilities.php‎

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2494,7 +2494,10 @@ public static function getCapabilities( array $input ): array { // phpcs:ignor
24942494
*/
24952495
public static function showRepo( array $input ): array|\WP_Error {
24962496
if ( RemoteWorkspaceBackend::should_handle() ) {
2497-
return ( new RemoteWorkspaceBackend() )->show($input['name'] ?? '');
2497+
$result = ( new RemoteWorkspaceBackend() )->show($input['name'] ?? '');
2498+
if ( ! self::shouldFallbackToLocalWorkspace($result) ) {
2499+
return $result;
2500+
}
24982501
}
24992502

25002503
$workspace = new Workspace();
@@ -3398,16 +3401,6 @@ public static function prRebase( array $input ): array|\WP_Error {
33983401
* @return array
33993402
*/
34003403
public static function worktreeAdd( array $input ): array|\WP_Error {
3401-
if ( RemoteWorkspaceBackend::should_handle() ) {
3402-
$result = ( new RemoteWorkspaceBackend() )->worktree_add(
3403-
$input['repo'] ?? '',
3404-
$input['branch'] ?? '',
3405-
$input['from'] ?? null
3406-
);
3407-
return self::decorate_remote_workspace_result('worktree_add', $result);
3408-
}
3409-
3410-
$workspace = new Workspace();
34113404
// Default inject_context=true; only false when explicitly provided.
34123405
$inject_context = array_key_exists('inject_context', $input) ? (bool) $input['inject_context'] : true;
34133406
// Default bootstrap=true; only false when explicitly provided.
@@ -3424,6 +3417,19 @@ public static function worktreeAdd( array $input ): array|\WP_Error {
34243417
if ( isset($input['task_ref']) && '' !== trim( (string) $input['task_ref']) ) {
34253418
$task['task_ref'] = (string) $input['task_ref'];
34263419
}
3420+
3421+
if ( RemoteWorkspaceBackend::should_handle() ) {
3422+
$result = ( new RemoteWorkspaceBackend() )->worktree_add(
3423+
$input['repo'] ?? '',
3424+
$input['branch'] ?? '',
3425+
$input['from'] ?? null
3426+
);
3427+
if ( ! self::shouldFallbackToLocalWorkspace($result) ) {
3428+
return self::decorate_remote_workspace_result('worktree_add', $result);
3429+
}
3430+
}
3431+
3432+
$workspace = new Workspace();
34273433
return $workspace->worktree_add(
34283434
$input['repo'] ?? '',
34293435
$input['branch'] ?? '',
@@ -3437,6 +3443,13 @@ public static function worktreeAdd( array $input ): array|\WP_Error {
34373443
);
34383444
}
34393445

3446+
/**
3447+
* Whether a remote-backend miss should be retried against local workspace discovery.
3448+
*/
3449+
private static function shouldFallbackToLocalWorkspace( mixed $result ): bool {
3450+
return is_wp_error($result) && in_array($result->get_error_code(), array( 'remote_workspace_repo_not_found', 'unsupported_remote_workspace_repo_argument' ), true);
3451+
}
3452+
34403453
/**
34413454
* Refresh a worktree's injected context from the originating site.
34423455
*

‎inc/Workspace/RemoteWorkspaceBackend.php‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1317,13 +1317,27 @@ private function resolve_repo( string $repo_name ): string|\WP_Error {
13171317
return (string) $state['repos'][ $repo_name ]['repo'];
13181318
}
13191319

1320-
if ( str_contains($repo_name, '/') ) {
1320+
if ( $this->looks_like_url_or_path($repo_name) ) {
1321+
return new \WP_Error('unsupported_remote_workspace_repo_argument', sprintf('Remote workspace worktree add requires a registered workspace name or owner/repo slug, not URL/path argument "%s".', $repo_name), array( 'status' => 400 ));
1322+
}
1323+
1324+
if ( preg_match('#^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$#', $repo_name) ) {
13211325
return $repo_name;
13221326
}
13231327

13241328
return new \WP_Error('remote_workspace_repo_not_found', sprintf('Remote workspace repository "%s" is not registered. Call workspace_clone first.', $repo_name), array( 'status' => 404 ));
13251329
}
13261330

1331+
private function looks_like_url_or_path( string $value ): bool {
1332+
$value = trim($value);
1333+
return str_starts_with($value, '/')
1334+
|| str_starts_with($value, './')
1335+
|| str_starts_with($value, '../')
1336+
|| str_starts_with($value, '~/')
1337+
|| (bool) preg_match('#^(?:https?|ssh|git)://#i', $value)
1338+
|| (bool) preg_match('/^[^@\s]+@[^:\s]+:.+$/', $value);
1339+
}
1340+
13271341
private function resolve_alias( string $handle ): string {
13281342
if ( class_exists(WorkspaceAliasResolver::class) ) {
13291343
return WorkspaceAliasResolver::resolve($handle);

‎inc/Workspace/WorkspaceCoreUtilities.php‎

Lines changed: 92 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -402,7 +402,98 @@ private function sanitize_slug( string $slug ): string {
402402
* @return string
403403
*/
404404
public function get_primary_path( string $repo ): string {
405-
return $this->workspace_path . '/' . $this->sanitize_name($repo);
405+
$resolved = $this->resolve_primary_repo_name($repo);
406+
if ( is_wp_error($resolved) ) {
407+
return $this->workspace_path . '/' . $this->sanitize_name($repo);
408+
}
409+
410+
return $this->workspace_path . '/' . $resolved;
411+
}
412+
413+
/**
414+
* Resolve a primary repo argument to the canonical workspace directory name.
415+
*
416+
* @param string $repo Primary handle, git URL, or local checkout path.
417+
* @return string|\WP_Error Canonical primary handle or validation error.
418+
*/
419+
public function resolve_primary_repo_name( string $repo ): string|\WP_Error {
420+
$repo = trim($repo);
421+
if ( '' === $repo ) {
422+
return new \WP_Error('invalid_repo', 'Repository name is required.', array( 'status' => 400 ));
423+
}
424+
425+
if ( str_contains($repo, '@') && ! $this->looks_like_git_url($repo) ) {
426+
return new \WP_Error('invalid_repo', 'Worktree handles cannot be used where a primary repository is required.', array( 'status' => 400 ));
427+
}
428+
429+
if ( $this->looks_like_git_url($repo) ) {
430+
$existing = $this->find_primary_by_remote($repo);
431+
if ( null !== $existing ) {
432+
return $existing['name'];
433+
}
434+
435+
return new \WP_Error('unsupported_workspace_repo_argument', sprintf('Repository URL "%s" does not match an existing local primary checkout. Use a registered primary handle or run workspace clone first.', $repo), array( 'status' => 404 ));
436+
}
437+
438+
if ( $this->looks_like_path_argument($repo) ) {
439+
return $this->resolve_primary_repo_name_from_path($repo);
440+
}
441+
442+
if ( str_contains($repo, '/') || str_contains($repo, '\\') ) {
443+
return new \WP_Error('unsupported_workspace_repo_argument', sprintf('Repository argument "%s" is not a primary workspace handle. Use the local primary handle, or pass a URL/path that matches an existing local primary checkout.', $repo), array( 'status' => 400 ));
444+
}
445+
446+
$sanitized = $this->sanitize_name($repo);
447+
if ( '' === $sanitized ) {
448+
return new \WP_Error('invalid_repo', sprintf('Repository argument "%s" did not produce a valid workspace handle.', $repo), array( 'status' => 400 ));
449+
}
450+
451+
return $sanitized;
452+
}
453+
454+
/**
455+
* Resolve a local path argument to an existing primary handle.
456+
*
457+
* @param string $path Local checkout path.
458+
* @return string|\WP_Error Canonical primary handle or validation error.
459+
*/
460+
private function resolve_primary_repo_name_from_path( string $path ): string|\WP_Error {
461+
$path = rtrim($path, '/');
462+
$expanded_path = str_starts_with($path, '~/') ? rtrim( (string) getenv('HOME'), '/') . substr($path, 1) : $path;
463+
$real_path = realpath($expanded_path);
464+
$workspace = realpath($this->workspace_path);
465+
$resolved_path = false !== $real_path ? $real_path : $expanded_path;
466+
467+
if ( false !== $workspace && str_starts_with(rtrim($resolved_path, '/') . '/', rtrim($workspace, '/') . '/') ) {
468+
$name = basename($resolved_path);
469+
if ( '' !== $name && ! str_contains($name, '@') && is_dir($resolved_path) && ( is_dir($resolved_path . '/.git') || is_file($resolved_path . '/.git') ) ) {
470+
return $name;
471+
}
472+
}
473+
474+
$remote = ( is_dir($resolved_path) && ( is_dir($resolved_path . '/.git') || is_file($resolved_path . '/.git') ) ) ? $this->git_get_remote($resolved_path) : null;
475+
if ( null !== $remote ) {
476+
$existing = $this->find_primary_by_remote($remote);
477+
if ( null !== $existing ) {
478+
return $existing['name'];
479+
}
480+
}
481+
482+
return new \WP_Error('unsupported_workspace_repo_argument', sprintf('Repository path "%s" does not resolve to an existing local primary checkout. Use a registered primary handle or run workspace clone/adopt first.', $path), array( 'status' => 404 ));
483+
}
484+
485+
/**
486+
* Whether a repo argument looks like a git URL.
487+
*/
488+
private function looks_like_git_url( string $value ): bool {
489+
return (bool) preg_match('#^(?:https?|ssh|git)://#i', $value) || (bool) preg_match('/^[^@\s]+@[^:\s]+:.+$/', $value);
490+
}
491+
492+
/**
493+
* Whether a repo argument looks like a filesystem path.
494+
*/
495+
private function looks_like_path_argument( string $value ): bool {
496+
return str_starts_with($value, '/') || str_starts_with($value, './') || str_starts_with($value, '../') || str_starts_with($value, '~/');
406497
}
407498

408499
/**

‎inc/Workspace/WorkspaceRepositoryLifecycle.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -667,6 +667,11 @@ public function show_repo( string $handle ): array|\WP_Error {
667667
$handle = $target;
668668
}
669669

670+
$resolved_handle = $this->resolve_primary_repo_name($handle);
671+
if ( ! is_wp_error($resolved_handle) ) {
672+
$handle = $resolved_handle;
673+
}
674+
670675
$parsed = $this->parse_handle($handle);
671676
$repo_path = $this->workspace_path . '/' . $parsed['dir_name'];
672677

‎inc/Workspace/WorkspaceWorktreeCleanupEngine.php‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2280,19 +2280,25 @@ private function remove_worktree_by_path( string $repo, string $branch, string $
22802280
}
22812281

22822282
$cmd = sprintf('worktree remove %s%s', $force ? '--force ' : '', escapeshellarg($real_path));
2283-
$result = $this->run_git($primary_path, $cmd);
2283+
$result = $this->run_git($primary_path, $cmd, self::CLEANUP_GIT_PROBE_TIMEOUT);
22842284

22852285
if ( is_wp_error($result) ) {
22862286
return $result;
22872287
}
22882288

2289-
// If the directory survived `git worktree remove` (can happen for
2290-
// locked worktrees, or when the worktree was already detached), prune
2291-
// the directory manually so cleanup is effective.
2289+
// `git worktree remove` is the destructive boundary. If Git reports
2290+
// success but the path survives, fail the row instead of falling back to
2291+
// an unbounded recursive delete that can wedge cleanup apply.
22922292
if ( is_dir($real_path) ) {
2293-
$escaped = escapeshellarg($real_path);
2294-
// phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.system_calls_exec
2295-
exec(sprintf('rm -rf %s 2>&1', $escaped));
2293+
return new \WP_Error(
2294+
'worktree_remove_incomplete',
2295+
sprintf('Git reported worktree removal success, but the directory still exists: %s', $real_path),
2296+
array(
2297+
'status' => 500,
2298+
'path' => $real_path,
2299+
'primary_path' => $primary_path,
2300+
)
2301+
);
22962302
}
22972303

22982304
WorktreeContextInjector::forget_metadata(basename($wt_path));

‎inc/Workspace/WorkspaceWorktreeLifecycle.php‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,11 @@ public function worktree_add( string $repo, string $branch, ?string $from = null
7272
return $visible;
7373
}
7474

75-
$repo = $this->sanitize_name($repo);
75+
$repo = $this->resolve_primary_repo_name($repo);
7676
$branch = trim($branch);
77+
if ( is_wp_error($repo) ) {
78+
return $repo;
79+
}
7780

7881
if ( '' === $repo ) {
7982
return new \WP_Error('invalid_repo', 'Repository name is required.', array( 'status' => 400 ));

‎tests/smoke-remote-workspace-backend.php‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,12 @@ function update_option( string $key, mixed $value, bool $autoload = true ): bool
180180
$assert('worktree add returns DMC handle', ! is_wp_error($worktree) && 'example@fix-example' === $worktree['handle']);
181181
$assert('worktree add backend result omits model-facing guidance', ! is_wp_error($worktree) && ! array_key_exists('next_required_tool', $worktree) && ! array_key_exists('next_required_args', $worktree));
182182

183+
$url_worktree = $backend->worktree_add('https://github.com/chubes4/example.git', 'fix/url-argument');
184+
$assert('worktree add rejects URL repo arguments in remote backend', is_wp_error($url_worktree) && 'unsupported_remote_workspace_repo_argument' === $url_worktree->get_error_code());
185+
186+
$path_worktree = $backend->worktree_add('/Users/chubes/Developer/example', 'fix/path-argument');
187+
$assert('worktree add rejects path repo arguments in remote backend', is_wp_error($path_worktree) && 'unsupported_remote_workspace_repo_argument' === $path_worktree->get_error_code());
188+
183189
update_option(
184190
'datamachine_code_workspace_aliases',
185191
array(

0 commit comments

Comments
 (0)