diff --git a/inc/Abilities/WorkspaceAbilities.php b/inc/Abilities/WorkspaceAbilities.php index 8374722f..c13f334d 100644 --- a/inc/Abilities/WorkspaceAbilities.php +++ b/inc/Abilities/WorkspaceAbilities.php @@ -2494,7 +2494,10 @@ public static function getCapabilities( array $input ): array { // phpcs:ignor */ public static function showRepo( array $input ): array|\WP_Error { if ( RemoteWorkspaceBackend::should_handle() ) { - return ( new RemoteWorkspaceBackend() )->show($input['name'] ?? ''); + $result = ( new RemoteWorkspaceBackend() )->show($input['name'] ?? ''); + if ( ! self::shouldFallbackToLocalWorkspace($result) ) { + return $result; + } } $workspace = new Workspace(); @@ -3398,16 +3401,6 @@ public static function prRebase( array $input ): array|\WP_Error { * @return array */ public static function worktreeAdd( array $input ): array|\WP_Error { - if ( RemoteWorkspaceBackend::should_handle() ) { - $result = ( new RemoteWorkspaceBackend() )->worktree_add( - $input['repo'] ?? '', - $input['branch'] ?? '', - $input['from'] ?? null - ); - return self::decorate_remote_workspace_result('worktree_add', $result); - } - - $workspace = new Workspace(); // Default inject_context=true; only false when explicitly provided. $inject_context = array_key_exists('inject_context', $input) ? (bool) $input['inject_context'] : true; // Default bootstrap=true; only false when explicitly provided. @@ -3424,6 +3417,19 @@ public static function worktreeAdd( array $input ): array|\WP_Error { if ( isset($input['task_ref']) && '' !== trim( (string) $input['task_ref']) ) { $task['task_ref'] = (string) $input['task_ref']; } + + if ( RemoteWorkspaceBackend::should_handle() ) { + $result = ( new RemoteWorkspaceBackend() )->worktree_add( + $input['repo'] ?? '', + $input['branch'] ?? '', + $input['from'] ?? null + ); + if ( ! self::shouldFallbackToLocalWorkspace($result) ) { + return self::decorate_remote_workspace_result('worktree_add', $result); + } + } + + $workspace = new Workspace(); return $workspace->worktree_add( $input['repo'] ?? '', $input['branch'] ?? '', @@ -3437,6 +3443,13 @@ public static function worktreeAdd( array $input ): array|\WP_Error { ); } + /** + * Whether a remote-backend miss should be retried against local workspace discovery. + */ + private static function shouldFallbackToLocalWorkspace( mixed $result ): bool { + return is_wp_error($result) && in_array($result->get_error_code(), array( 'remote_workspace_repo_not_found', 'unsupported_remote_workspace_repo_argument' ), true); + } + /** * Refresh a worktree's injected context from the originating site. * diff --git a/inc/Workspace/RemoteWorkspaceBackend.php b/inc/Workspace/RemoteWorkspaceBackend.php index 32400d46..40735108 100644 --- a/inc/Workspace/RemoteWorkspaceBackend.php +++ b/inc/Workspace/RemoteWorkspaceBackend.php @@ -1317,13 +1317,27 @@ private function resolve_repo( string $repo_name ): string|\WP_Error { return (string) $state['repos'][ $repo_name ]['repo']; } - if ( str_contains($repo_name, '/') ) { + if ( $this->looks_like_url_or_path($repo_name) ) { + 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 )); + } + + if ( preg_match('#^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$#', $repo_name) ) { return $repo_name; } 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 )); } + private function looks_like_url_or_path( string $value ): bool { + $value = trim($value); + return str_starts_with($value, '/') + || str_starts_with($value, './') + || str_starts_with($value, '../') + || str_starts_with($value, '~/') + || (bool) preg_match('#^(?:https?|ssh|git)://#i', $value) + || (bool) preg_match('/^[^@\s]+@[^:\s]+:.+$/', $value); + } + private function resolve_alias( string $handle ): string { if ( class_exists(WorkspaceAliasResolver::class) ) { return WorkspaceAliasResolver::resolve($handle); diff --git a/inc/Workspace/WorkspaceCoreUtilities.php b/inc/Workspace/WorkspaceCoreUtilities.php index 89732156..7162a789 100644 --- a/inc/Workspace/WorkspaceCoreUtilities.php +++ b/inc/Workspace/WorkspaceCoreUtilities.php @@ -402,7 +402,98 @@ private function sanitize_slug( string $slug ): string { * @return string */ public function get_primary_path( string $repo ): string { - return $this->workspace_path . '/' . $this->sanitize_name($repo); + $resolved = $this->resolve_primary_repo_name($repo); + if ( is_wp_error($resolved) ) { + return $this->workspace_path . '/' . $this->sanitize_name($repo); + } + + return $this->workspace_path . '/' . $resolved; + } + + /** + * Resolve a primary repo argument to the canonical workspace directory name. + * + * @param string $repo Primary handle, git URL, or local checkout path. + * @return string|\WP_Error Canonical primary handle or validation error. + */ + public function resolve_primary_repo_name( string $repo ): string|\WP_Error { + $repo = trim($repo); + if ( '' === $repo ) { + return new \WP_Error('invalid_repo', 'Repository name is required.', array( 'status' => 400 )); + } + + if ( str_contains($repo, '@') && ! $this->looks_like_git_url($repo) ) { + return new \WP_Error('invalid_repo', 'Worktree handles cannot be used where a primary repository is required.', array( 'status' => 400 )); + } + + if ( $this->looks_like_git_url($repo) ) { + $existing = $this->find_primary_by_remote($repo); + if ( null !== $existing ) { + return $existing['name']; + } + + 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 )); + } + + if ( $this->looks_like_path_argument($repo) ) { + return $this->resolve_primary_repo_name_from_path($repo); + } + + if ( str_contains($repo, '/') || str_contains($repo, '\\') ) { + 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 )); + } + + $sanitized = $this->sanitize_name($repo); + if ( '' === $sanitized ) { + return new \WP_Error('invalid_repo', sprintf('Repository argument "%s" did not produce a valid workspace handle.', $repo), array( 'status' => 400 )); + } + + return $sanitized; + } + + /** + * Resolve a local path argument to an existing primary handle. + * + * @param string $path Local checkout path. + * @return string|\WP_Error Canonical primary handle or validation error. + */ + private function resolve_primary_repo_name_from_path( string $path ): string|\WP_Error { + $path = rtrim($path, '/'); + $expanded_path = str_starts_with($path, '~/') ? rtrim( (string) getenv('HOME'), '/') . substr($path, 1) : $path; + $real_path = realpath($expanded_path); + $workspace = realpath($this->workspace_path); + $resolved_path = false !== $real_path ? $real_path : $expanded_path; + + if ( false !== $workspace && str_starts_with(rtrim($resolved_path, '/') . '/', rtrim($workspace, '/') . '/') ) { + $name = basename($resolved_path); + if ( '' !== $name && ! str_contains($name, '@') && is_dir($resolved_path) && ( is_dir($resolved_path . '/.git') || is_file($resolved_path . '/.git') ) ) { + return $name; + } + } + + $remote = ( is_dir($resolved_path) && ( is_dir($resolved_path . '/.git') || is_file($resolved_path . '/.git') ) ) ? $this->git_get_remote($resolved_path) : null; + if ( null !== $remote ) { + $existing = $this->find_primary_by_remote($remote); + if ( null !== $existing ) { + return $existing['name']; + } + } + + 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 )); + } + + /** + * Whether a repo argument looks like a git URL. + */ + private function looks_like_git_url( string $value ): bool { + return (bool) preg_match('#^(?:https?|ssh|git)://#i', $value) || (bool) preg_match('/^[^@\s]+@[^:\s]+:.+$/', $value); + } + + /** + * Whether a repo argument looks like a filesystem path. + */ + private function looks_like_path_argument( string $value ): bool { + return str_starts_with($value, '/') || str_starts_with($value, './') || str_starts_with($value, '../') || str_starts_with($value, '~/'); } /** diff --git a/inc/Workspace/WorkspaceRepositoryLifecycle.php b/inc/Workspace/WorkspaceRepositoryLifecycle.php index b0036c5c..b6202522 100644 --- a/inc/Workspace/WorkspaceRepositoryLifecycle.php +++ b/inc/Workspace/WorkspaceRepositoryLifecycle.php @@ -667,6 +667,11 @@ public function show_repo( string $handle ): array|\WP_Error { $handle = $target; } + $resolved_handle = $this->resolve_primary_repo_name($handle); + if ( ! is_wp_error($resolved_handle) ) { + $handle = $resolved_handle; + } + $parsed = $this->parse_handle($handle); $repo_path = $this->workspace_path . '/' . $parsed['dir_name']; diff --git a/inc/Workspace/WorkspaceWorktreeCleanupEngine.php b/inc/Workspace/WorkspaceWorktreeCleanupEngine.php index 846cd021..a36b8915 100644 --- a/inc/Workspace/WorkspaceWorktreeCleanupEngine.php +++ b/inc/Workspace/WorkspaceWorktreeCleanupEngine.php @@ -2280,19 +2280,25 @@ private function remove_worktree_by_path( string $repo, string $branch, string $ } $cmd = sprintf('worktree remove %s%s', $force ? '--force ' : '', escapeshellarg($real_path)); - $result = $this->run_git($primary_path, $cmd); + $result = $this->run_git($primary_path, $cmd, self::CLEANUP_GIT_PROBE_TIMEOUT); if ( is_wp_error($result) ) { return $result; } - // If the directory survived `git worktree remove` (can happen for - // locked worktrees, or when the worktree was already detached), prune - // the directory manually so cleanup is effective. + // `git worktree remove` is the destructive boundary. If Git reports + // success but the path survives, fail the row instead of falling back to + // an unbounded recursive delete that can wedge cleanup apply. if ( is_dir($real_path) ) { - $escaped = escapeshellarg($real_path); - // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.system_calls_exec - exec(sprintf('rm -rf %s 2>&1', $escaped)); + return new \WP_Error( + 'worktree_remove_incomplete', + sprintf('Git reported worktree removal success, but the directory still exists: %s', $real_path), + array( + 'status' => 500, + 'path' => $real_path, + 'primary_path' => $primary_path, + ) + ); } WorktreeContextInjector::forget_metadata(basename($wt_path)); diff --git a/inc/Workspace/WorkspaceWorktreeLifecycle.php b/inc/Workspace/WorkspaceWorktreeLifecycle.php index 2d7fb3d0..9aefe267 100644 --- a/inc/Workspace/WorkspaceWorktreeLifecycle.php +++ b/inc/Workspace/WorkspaceWorktreeLifecycle.php @@ -72,8 +72,11 @@ public function worktree_add( string $repo, string $branch, ?string $from = null return $visible; } - $repo = $this->sanitize_name($repo); + $repo = $this->resolve_primary_repo_name($repo); $branch = trim($branch); + if ( is_wp_error($repo) ) { + return $repo; + } if ( '' === $repo ) { return new \WP_Error('invalid_repo', 'Repository name is required.', array( 'status' => 400 )); diff --git a/tests/smoke-remote-workspace-backend.php b/tests/smoke-remote-workspace-backend.php index a0c8c29c..434642be 100644 --- a/tests/smoke-remote-workspace-backend.php +++ b/tests/smoke-remote-workspace-backend.php @@ -180,6 +180,12 @@ function update_option( string $key, mixed $value, bool $autoload = true ): bool $assert('worktree add returns DMC handle', ! is_wp_error($worktree) && 'example@fix-example' === $worktree['handle']); $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)); + $url_worktree = $backend->worktree_add('https://github.com/chubes4/example.git', 'fix/url-argument'); + $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()); + + $path_worktree = $backend->worktree_add('/Users/chubes/Developer/example', 'fix/path-argument'); + $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()); + update_option( 'datamachine_code_workspace_aliases', array( diff --git a/tests/smoke-workspace-local-fallback.php b/tests/smoke-workspace-local-fallback.php new file mode 100644 index 00000000..6399825e --- /dev/null +++ b/tests/smoke-workspace-local-fallback.php @@ -0,0 +1,190 @@ + true, + 'name' => $handle, + 'repo' => $handle, + 'is_worktree' => false, + 'path' => '/Users/chubes/Developer/' . $handle, + 'branch' => 'main', + 'remote' => 'https://github.com/Extra-Chill/homeboy.git', + 'commit' => 'abc123 listed primary', + 'dirty' => 0, + ); + } + + public function worktree_add( + string $repo, + string $branch, + ?string $from = null, + bool $inject_context = true, + bool $bootstrap = true, + bool $allow_stale = false, + bool $rebase_base = false, + bool $force = false, + array $task = array() + ): array { + self::$worktree_input = compact('repo', 'branch', 'from', 'inject_context', 'bootstrap', 'allow_stale', 'rebase_base', 'force', 'task'); + return array( + 'success' => true, + 'handle' => $repo . '@' . str_replace('/', '-', $branch), + 'path' => '/Users/chubes/Developer/' . $repo . '@' . str_replace('/', '-', $branch), + ); + } + } +} + +namespace DataMachineCode\Support { + class RuntimeCapabilities + { + } + + class GitRunner + { + } +} + +namespace { + if ( ! defined('ABSPATH') ) { + define('ABSPATH', __DIR__); + } + + class WP_Error + { + public function __construct( private string $code, private string $message, private array $data = array() ) + { + } + + public function get_error_code(): string + { + return $this->code; + } + + public function get_error_message(): string + { + return $this->message; + } + + public function get_error_data(): array + { + return $this->data; + } + } + + function is_wp_error( $value ): bool + { + return $value instanceof WP_Error; + } + + require __DIR__ . '/../inc/Abilities/WorkspaceAbilities.php'; + + $failures = array(); + $assert = function ( string $label, bool $condition ) use ( &$failures ): void { + if ( $condition ) { + echo " ok {$label}\n"; + return; + } + + $failures[] = $label; + echo " fail {$label}\n"; + }; + + echo "Workspace local fallback - smoke\n"; + + $show = \DataMachineCode\Abilities\WorkspaceAbilities::showRepo( + array( + 'name' => 'homeboy', + ) + ); + + $assert('remote show attempted first', 'homeboy' === ( \DataMachineCode\Workspace\RemoteWorkspaceBackend::$show_input['handle'] ?? '' )); + $assert('local show fallback attempted', 'homeboy' === ( \DataMachineCode\Workspace\Workspace::$show_input['handle'] ?? '' )); + $assert('show returns local listed primary', is_array($show) && '/Users/chubes/Developer/homeboy' === ( $show['path'] ?? '' )); + + $worktree = \DataMachineCode\Abilities\WorkspaceAbilities::worktreeAdd( + array( + 'repo' => 'homeboy', + 'branch' => 'fix/issue-4072-artifact-viewer-urls', + 'from' => 'origin/main', + 'inject_context' => false, + 'bootstrap' => false, + 'allow_stale' => true, + 'rebase_base' => true, + 'force' => true, + 'task_url' => 'https://github.com/Extra-Chill/homeboy/issues/4072', + 'task_ref' => 'Extra-Chill/homeboy#4072', + ) + ); + + $assert('remote worktree add attempted first', 'homeboy' === ( \DataMachineCode\Workspace\RemoteWorkspaceBackend::$worktree_input['repo_name'] ?? '' )); + $assert('local worktree add fallback attempted', 'homeboy' === ( \DataMachineCode\Workspace\Workspace::$worktree_input['repo'] ?? '' )); + $assert('worktree add preserves base ref', 'origin/main' === ( \DataMachineCode\Workspace\Workspace::$worktree_input['from'] ?? '' )); + $assert('worktree add preserves local options', false === ( \DataMachineCode\Workspace\Workspace::$worktree_input['inject_context'] ?? null ) && true === ( \DataMachineCode\Workspace\Workspace::$worktree_input['allow_stale'] ?? null )); + $assert('worktree add preserves task metadata', 'Extra-Chill/homeboy#4072' === ( \DataMachineCode\Workspace\Workspace::$worktree_input['task']['task_ref'] ?? '' )); + $assert('worktree add returns local worktree result', is_array($worktree) && 'homeboy@fix-issue-4072-artifact-viewer-urls' === ( $worktree['handle'] ?? '' )); + + if ( $failures ) { + echo "\nFailures:\n"; + foreach ( $failures as $failure ) { + echo " - {$failure}\n"; + } + exit(1); + } + + echo "\nOK\n"; +} diff --git a/tests/smoke-worktree-cleanup-remove-guard.php b/tests/smoke-worktree-cleanup-remove-guard.php new file mode 100644 index 00000000..54ad3e6a --- /dev/null +++ b/tests/smoke-worktree-cleanup-remove-guard.php @@ -0,0 +1,46 @@ +run_git($primary_path, $cmd, self::CLEANUP_GIT_PROBE_TIMEOUT)') ) { + $failures[] = 'git worktree remove must use the cleanup git timeout'; +} + +if ( str_contains($function, 'rm -rf') ) { + $failures[] = 'remove_worktree_by_path must not fall back to rm -rf'; +} + +if ( ! str_contains($function, 'worktree_remove_incomplete') ) { + $failures[] = 'surviving worktree directory must return a row-level failure'; +} + +if ( $failures ) { + echo "Worktree cleanup remove guard failed:\n"; + foreach ( $failures as $failure ) { + echo " - {$failure}\n"; + } + exit(1); +} + +echo "Worktree cleanup remove guard OK\n"; diff --git a/tests/smoke-worktree-lifecycle-metadata.php b/tests/smoke-worktree-lifecycle-metadata.php index 4097929b..686f84dc 100644 --- a/tests/smoke-worktree-lifecycle-metadata.php +++ b/tests/smoke-worktree-lifecycle-metadata.php @@ -81,6 +81,11 @@ public function get_error_message(): string return $this->message; } + public function get_error_code(): string + { + return $this->code; + } + public function get_error_data() { return $this->data; @@ -264,6 +269,7 @@ function get_userdata( int $user_id ): object include __DIR__ . '/../inc/Workspace/WorkspaceMutationLock.php'; include __DIR__ . '/../inc/Workspace/WorktreeStalenessProbe.php'; include __DIR__ . '/../inc/Workspace/WorktreeDiskBudget.php'; + include __DIR__ . '/../inc/Workspace/WorktreeBootstrapper.php'; include __DIR__ . '/../inc/Workspace/WorktreeContextInjector.php'; include __DIR__ . '/../inc/Workspace/Workspace.php'; @@ -341,9 +347,21 @@ function () use ( $tmp ): void { $linked_items = array_values(array_filter($linked_list['worktrees'] ?? array(), fn( $wt ) => ( $wt['handle'] ?? '' ) === 'linked-primary')); $assert(1, count($linked_items), 'worktree_list discovers primary checkouts whose .git is a file'); + $url_worktree = $ws->worktree_add('https://github.com/acme/demo.git', 'feature/url-primary-resolution', 'HEAD', false, false, true, false, false); + $assert(false, is_wp_error($url_worktree), 'worktree_add accepts URL matching an existing local primary'); + $assert('demo@feature-url-primary-resolution', is_wp_error($url_worktree) ? null : ( $url_worktree['handle'] ?? null ), 'URL repo argument normalizes to primary handle'); + + $path_worktree = $ws->worktree_add($primary, 'feature/path-primary-resolution', 'HEAD', false, false, true, false, false); + $assert(false, is_wp_error($path_worktree), 'worktree_add accepts path to an existing local primary'); + $assert('demo@feature-path-primary-resolution', is_wp_error($path_worktree) ? null : ( $path_worktree['handle'] ?? null ), 'path repo argument normalizes to primary handle'); + + $bad_url_worktree = $ws->worktree_add('https://github.com/acme/missing.git', 'feature/missing-url-primary-resolution', 'HEAD', false, false, true, false, false); + $assert(true, is_wp_error($bad_url_worktree), 'worktree_add rejects URL repo arguments without a matching local primary'); + $assert('unsupported_workspace_repo_argument', is_wp_error($bad_url_worktree) ? $bad_url_worktree->get_error_code() : null, 'missing URL primary returns a clear error code'); + $GLOBALS['datamachine_code_test_filters']['datamachine_worktree_disk_budget_thresholds'] = function ( array $thresholds ): array { - $thresholds['refuse_free_bytes'] = PHP_INT_MAX; - $thresholds['warn_free_bytes'] = PHP_INT_MAX; + $thresholds['refuse_free_percent'] = 100; + $thresholds['warn_free_percent'] = 100; return $thresholds; }; $refused = $ws->worktree_add('demo', 'feature/disk-budget-refusal', 'HEAD', true, true, true, false, false);