From 2a02f00bad562756414c231d0169304dd1062f74 Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Fri, 24 Apr 2026 12:57:08 -0400 Subject: [PATCH] feat(workspace): surface worktree staleness at create-time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit refs #52 `worktree add` now unconditionally fetches `origin` before creating the new checkout and then compares the materialized branch (or the local base it was cut from) against its origin counterpart, surfacing the behind-count in the ability response and CLI output. Agents can decide whether to rebase before cooking instead of discovering day-one merge conflicts at PR-review time. Non-breaking: no gating, no auto-rebase, all new response fields are optional, CLI warns but never fails. The `--allow-stale` + `--rebase-base` behavior change lives in a follow-up PR (option 2 in #52). Changes: - `WorktreeStalenessProbe` helper — fetch + behind-count + output parsing, returns structured results instead of throwing (offline work still possible) - `Workspace::worktree_add()` — unconditional fetch at the top, post-creation staleness computation for both the existing-local-branch path and the new-branch-off-local-base path - `is_remote_tracking_ref()` helper — recognizes `refs/remotes/origin/*` (what `resolve_default_base()` returns) AND `origin/*` short form as "already at-tip post-fetch" - `WorkspaceAbilities` — output schema gains six optional fields: `fetch_failed`, `fetch_error`, `stale_commits_behind`, `upstream`, `base_stale_commits_behind`, `base_upstream` - `WorkspaceCommand::render_worktree_freshness()` — renders a `Freshness:` block after bootstrap output. Elides the line entirely when no signal is available rather than print a misleading "up to date" - `tests/smoke-worktree-staleness.php` — 23 assertions: parse_count, is_missing_upstream heuristics, real-git fixtures for fetch success, fetch failure (no origin), behind-count parsing, no-upstream → null, at-tip → 0 --- inc/Abilities/WorkspaceAbilities.php | 24 +++ inc/Cli/Commands/WorkspaceCommand.php | 66 ++++++++ inc/Workspace/Workspace.php | 75 ++++++++- inc/Workspace/WorktreeStalenessProbe.php | 129 ++++++++++++++ tests/smoke-worktree-staleness.php | 204 +++++++++++++++++++++++ 5 files changed, 493 insertions(+), 5 deletions(-) create mode 100644 inc/Workspace/WorktreeStalenessProbe.php create mode 100644 tests/smoke-worktree-staleness.php diff --git a/inc/Abilities/WorkspaceAbilities.php b/inc/Abilities/WorkspaceAbilities.php index 7a46efd..26929a9 100644 --- a/inc/Abilities/WorkspaceAbilities.php +++ b/inc/Abilities/WorkspaceAbilities.php @@ -755,6 +755,30 @@ private function registerAbilities(): void { 'type' => 'object', 'description' => 'Present only when bootstrap=true. Contains success/ran_any booleans and a steps array.', ), + 'fetch_failed' => array( + 'type' => 'boolean', + 'description' => 'Present only when the pre-create `git fetch origin` failed. Worktree creation continues either way; staleness fields are omitted when true.', + ), + 'fetch_error' => array( + 'type' => 'string', + 'description' => 'Present only when fetch_failed=true. Trimmed error output from the failing fetch.', + ), + 'stale_commits_behind' => array( + 'type' => 'integer', + 'description' => 'For the existing-local-branch path, how many commits the worktree branch is behind its configured upstream. Omitted when no upstream is configured.', + ), + 'upstream' => array( + 'type' => 'string', + 'description' => 'Paired with stale_commits_behind: the upstream ref label (e.g. `origin/fix/foo`).', + ), + 'base_stale_commits_behind' => array( + 'type' => 'integer', + 'description' => 'For the new-branch path cut from a local base ref: how many commits that local base is behind its origin counterpart at fetch time.', + ), + 'base_upstream' => array( + 'type' => 'string', + 'description' => 'Paired with base_stale_commits_behind: the origin ref the local base was compared against (e.g. `origin/main`).', + ), ), ), 'execute_callback' => array( self::class, 'worktreeAdd' ), diff --git a/inc/Cli/Commands/WorkspaceCommand.php b/inc/Cli/Commands/WorkspaceCommand.php index a3010c0..41f4b9f 100644 --- a/inc/Cli/Commands/WorkspaceCommand.php +++ b/inc/Cli/Commands/WorkspaceCommand.php @@ -1188,6 +1188,7 @@ private function renderWorktreeResult( string $operation, array $result, array $ WP_CLI::warning( 'Worktree was created but bootstrap had failures. Re-run the failing step manually, or remove and retry.' ); } } + $this->render_worktree_freshness( $result ); return; case 'refresh-context': @@ -1211,4 +1212,69 @@ private function renderWorktreeResult( string $operation, array $result, array $ return; } } + + /** + * Render the freshness block for `worktree add` results. + * + * States, in priority order: + * - fetch_failed=true → `⚠ fetch failed — staleness unknown` (warning) + * - stale_commits_behind>0 → `⚠ commits behind ` (warning + rebase hint) + * - base_stale_commits_behind>0 → `⚠ base was commits behind ` (warning + rebase hint) + * - otherwise → `Freshness: up to date` (log) + * + * When no staleness signal is present at all (no fetch attempt recorded, + * no upstream configured, defaults used) the line is elided entirely — + * silence beats an ambiguous "up to date" we can't actually vouch for. + * + * @param array $result Ability result payload. + * @return void + */ + private function render_worktree_freshness( array $result ): void { + if ( ! empty( $result['fetch_failed'] ) ) { + $msg = 'Freshness: ⚠ fetch failed — staleness unknown'; + if ( ! empty( $result['fetch_error'] ) ) { + $msg .= "\n " . $result['fetch_error']; + } + WP_CLI::warning( $msg ); + return; + } + + if ( isset( $result['stale_commits_behind'] ) ) { + $behind = (int) $result['stale_commits_behind']; + $upstream = isset( $result['upstream'] ) ? (string) $result['upstream'] : 'upstream'; + if ( $behind > 0 ) { + WP_CLI::warning( sprintf( + "Freshness: ⚠ %d commits behind %s\n Rebase before opening a PR:\n git -C %s pull --rebase origin %s", + $behind, + $upstream, + $result['path'] ?? '', + $result['branch'] ?? '' + ) ); + return; + } + WP_CLI::log( sprintf( 'Freshness: up to date (vs %s)', $upstream ) ); + return; + } + + if ( isset( $result['base_stale_commits_behind'] ) ) { + $behind = (int) $result['base_stale_commits_behind']; + $base_upstream = isset( $result['base_upstream'] ) ? (string) $result['base_upstream'] : 'origin'; + if ( $behind > 0 ) { + WP_CLI::warning( sprintf( + "Freshness: ⚠ base was %d commits behind %s\n Rebase before opening a PR:\n git -C %s pull --rebase origin %s", + $behind, + $base_upstream, + $result['path'] ?? '', + $result['branch'] ?? '' + ) ); + return; + } + WP_CLI::log( sprintf( 'Freshness: up to date (base %s)', $base_upstream ) ); + return; + } + + // No signal available (default base was origin/HEAD, or no upstream + // configured for the existing branch). Elide the line rather than + // print a potentially-misleading "up to date". + } } diff --git a/inc/Workspace/Workspace.php b/inc/Workspace/Workspace.php index 804e1be..74b0ea6 100644 --- a/inc/Workspace/Workspace.php +++ b/inc/Workspace/Workspace.php @@ -876,7 +876,7 @@ public function git_diff( string $name, ?string $from = null, ?string $to = null * @param string|null $from Base ref when creating the branch. * @param bool $inject_context Whether to inject site-agent context (default true). * @param bool $bootstrap Whether to run submodule/package/composer install after creation (default true). - * @return array{success: bool, handle: string, path: string, branch: string, slug: string, created_branch: bool, message: string, context_injected?: bool, context_files?: string[], context_skip_reason?: string, bootstrap?: array}|\WP_Error + * @return array{success: bool, handle: string, path: string, branch: string, slug: string, created_branch: bool, message: string, context_injected?: bool, context_files?: string[], context_skip_reason?: string, bootstrap?: array, fetch_failed?: bool, fetch_error?: string, stale_commits_behind?: int, upstream?: string, base_stale_commits_behind?: int, base_upstream?: string}|\WP_Error */ public function worktree_add( string $repo, string $branch, ?string $from = null, bool $inject_context = true, bool $bootstrap = true ): array|\WP_Error { $repo = $this->sanitize_name( $repo ); @@ -907,18 +907,25 @@ public function worktree_add( string $repo, string $branch, ?string $from = null return new \WP_Error( 'worktree_exists', sprintf( 'Worktree handle "%s" already exists.', $wt_handle ), array( 'status' => 400 ) ); } + // Always fetch first so staleness data (and the default base) reflects the + // current remote. Failure is logged but never aborts — offline work should + // still be possible, the agent just needs to know staleness is unknown. + $fetch = WorktreeStalenessProbe::fetch( $primary_path ); + $fetch_failed = ! $fetch['ok']; + $fetch_error = $fetch['error'] ?? null; + // Does the branch already exist locally? // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.system_calls_exec exec( sprintf( 'git -C %s show-ref --verify --quiet %s 2>&1', escapeshellarg( $primary_path ), escapeshellarg( 'refs/heads/' . $branch ) ), $_unused, $exists_local ); $created_branch = false; + $resolved_base = null; if ( 0 === $exists_local ) { $cmd = sprintf( 'worktree add %s %s', escapeshellarg( $wt_path ), escapeshellarg( $branch ) ); } else { - $base = $from && '' !== trim( $from ) ? trim( $from ) : $this->resolve_default_base( $primary_path ); - // Fetch first to make sure remote refs are current. - $this->run_git( $primary_path, 'fetch --quiet origin' ); - $cmd = sprintf( 'worktree add -b %s %s %s', escapeshellarg( $branch ), escapeshellarg( $wt_path ), escapeshellarg( $base ) ); + $base = $from && '' !== trim( $from ) ? trim( $from ) : $this->resolve_default_base( $primary_path ); + $resolved_base = $base; + $cmd = sprintf( 'worktree add -b %s %s %s', escapeshellarg( $branch ), escapeshellarg( $wt_path ), escapeshellarg( $base ) ); $created_branch = true; } @@ -937,6 +944,49 @@ public function worktree_add( string $repo, string $branch, ?string $from = null 'message' => sprintf( 'Worktree "%s" added at %s (branch %s).', $wt_handle, $wt_path, $branch ), ); + if ( $fetch_failed ) { + $response['fetch_failed'] = true; + if ( null !== $fetch_error && '' !== $fetch_error ) { + $response['fetch_error'] = $fetch_error; + } + } + + // Compute staleness. Only meaningful when fetch succeeded — otherwise the + // upstream refs are potentially stale themselves and any behind-count we + // produce would be misleading. + if ( ! $fetch_failed ) { + if ( ! $created_branch ) { + // Existing local branch: compare against its configured upstream. + $behind = WorktreeStalenessProbe::behind_count( $wt_path, $branch, '@{upstream}' ); + if ( is_int( $behind ) ) { + $response['stale_commits_behind'] = $behind; + // Derive a human-readable upstream label. Best-effort; silently + // skipped when git's plumbing doesn't cooperate. + $upstream_name = $this->run_git( + $wt_path, + sprintf( 'rev-parse --abbrev-ref --symbolic-full-name %s', escapeshellarg( $branch . '@{upstream}' ) ) + ); + if ( ! is_wp_error( $upstream_name ) ) { + $label = trim( (string) ( $upstream_name['output'] ?? '' ) ); + if ( '' !== $label ) { + $response['upstream'] = $label; + } + } + } + // null → no upstream configured; WP_Error → unexpected failure. + // Both cases: silently omit staleness fields. + } elseif ( null !== $resolved_base && ! $this->is_remote_tracking_ref( $resolved_base ) && 'HEAD' !== $resolved_base ) { + // New branch cut from a local ref: compare that ref to its origin + // counterpart so the agent sees when the base itself was stale. + $base_upstream = 'origin/' . $resolved_base; + $behind = WorktreeStalenessProbe::behind_count( $primary_path, $resolved_base, $base_upstream ); + if ( is_int( $behind ) ) { + $response['base_stale_commits_behind'] = $behind; + $response['base_upstream'] = $base_upstream; + } + } + } + if ( ! $inject_context ) { $response['context_injected'] = false; $response['context_skip_reason'] = 'inject_context flag disabled'; @@ -1613,6 +1663,21 @@ private function resolve_default_base( string $repo_path ): string { return 'HEAD'; } + /** + * Does a ref look like a remote-tracking ref? + * + * `resolve_default_base()` returns fully-qualified paths + * (`refs/remotes/origin/main`), but callers may pass short forms like + * `origin/main`. Both are "already at-tip post-fetch" and staleness + * comparisons against them would be nonsensical. + * + * @param string $ref Ref name to classify. + * @return bool + */ + private function is_remote_tracking_ref( string $ref ): bool { + return str_starts_with( $ref, 'refs/remotes/' ) || str_starts_with( $ref, 'origin/' ); + } + /** * Parse a `git worktree list --porcelain` block. * diff --git a/inc/Workspace/WorktreeStalenessProbe.php b/inc/Workspace/WorktreeStalenessProbe.php new file mode 100644 index 0000000..856d939 --- /dev/null +++ b/inc/Workspace/WorktreeStalenessProbe.php @@ -0,0 +1,129 @@ +get_error_data(); + $tail = is_array( $data ) && isset( $data['output'] ) ? trim( (string) $data['output'] ) : ''; + $error = '' !== $tail ? $tail : $result->get_error_message(); + return array( + 'ok' => false, + 'error' => $error, + ); + } + return array( 'ok' => true ); + } + + /** + * Count commits `$ref` is behind `$upstream` via `git rev-list --count`. + * + * Returns an integer ≥ 0 on success, null when `$upstream` is not + * configured / does not exist, and a WP_Error on any other git failure + * so the caller can surface it without conflating "no upstream" with + * "command errored". + * + * @param string $repo_path Repository path (worktree path or primary). + * @param string $ref Left-hand revision (e.g. current branch name). + * @param string $upstream Right-hand revision (e.g. `@{upstream}` or `origin/main`). + * @return int|null|\WP_Error + */ + public static function behind_count( string $repo_path, string $ref, string $upstream ): int|null|\WP_Error { + $args = sprintf( 'rev-list --count %s..%s', escapeshellarg( $ref ), escapeshellarg( $upstream ) ); + $result = GitRunner::run( $repo_path, $args ); + if ( is_wp_error( $result ) ) { + $data = $result->get_error_data(); + $out = is_array( $data ) && isset( $data['output'] ) ? (string) $data['output'] : ''; + + // Missing upstream configuration. Treated as "unknown", not an error. + if ( self::is_missing_upstream( $out ) ) { + return null; + } + return $result; + } + + $count = self::parse_count( (string) ( $result['output'] ?? '' ) ); + return $count; + } + + /** + * Parse a `rev-list --count` stdout payload into an int. Tolerant of + * trailing whitespace / empty output (returns 0 for empty, matching git). + * + * @param string $output Raw stdout. + * @return int + */ + public static function parse_count( string $output ): int { + $trimmed = trim( $output ); + if ( '' === $trimmed ) { + return 0; + } + if ( ! preg_match( '/^\d+$/', $trimmed ) ) { + return 0; + } + return (int) $trimmed; + } + + /** + * Heuristic: does a git error blob signal "no upstream configured" rather + * than a real failure? Matches the common phrasings git uses across + * versions. + * + * @param string $output Git stderr/stdout. + * @return bool + */ + public static function is_missing_upstream( string $output ): bool { + $needles = array( + 'no upstream configured', + 'unknown revision', + 'bad revision', + 'ambiguous argument', + 'No such ref', + 'Needed a single revision', + ); + foreach ( $needles as $needle ) { + if ( false !== stripos( $output, $needle ) ) { + return true; + } + } + return false; + } +} diff --git a/tests/smoke-worktree-staleness.php b/tests/smoke-worktree-staleness.php new file mode 100644 index 0000000..7e58168 --- /dev/null +++ b/tests/smoke-worktree-staleness.php @@ -0,0 +1,204 @@ +code = $code; + $this->message = $message; + $this->data = $data; + } + public function get_error_code() { return $this->code; } + public function get_error_message() { return $this->message; } + public function get_error_data() { return $this->data; } + } +} + +if ( ! function_exists( 'is_wp_error' ) ) { + function is_wp_error( $thing ): bool { + return $thing instanceof WP_Error; + } +} + +require __DIR__ . '/../inc/Support/GitRunner.php'; +require __DIR__ . '/../inc/Workspace/WorktreeStalenessProbe.php'; + +use DataMachineCode\Workspace\WorktreeStalenessProbe; + +$failures = 0; +$total = 0; + +$assert = function ( $expected, $actual, string $message ) use ( &$failures, &$total ): void { + $total++; + if ( $expected === $actual ) { + echo " ✓ {$message}\n"; + return; + } + $failures++; + echo " ✗ {$message}\n"; + echo " expected: " . var_export( $expected, true ) . "\n"; + echo " actual: " . var_export( $actual, true ) . "\n"; +}; + +$assert_true = function ( $condition, string $message ) use ( &$failures, &$total ): void { + $total++; + if ( $condition ) { + echo " ✓ {$message}\n"; + return; + } + $failures++; + echo " ✗ {$message}\n"; +}; + +$cleanup = function ( string $path ): void { + if ( ! is_dir( $path ) && ! is_file( $path ) ) { + return; + } + if ( is_file( $path ) ) { + unlink( $path ); + return; + } + $it = new RecursiveIteratorIterator( + new RecursiveDirectoryIterator( $path, FilesystemIterator::SKIP_DOTS ), + RecursiveIteratorIterator::CHILD_FIRST + ); + foreach ( $it as $entry ) { + $entry->isDir() ? rmdir( $entry->getPathname() ) : unlink( $entry->getPathname() ); + } + rmdir( $path ); +}; + +// ----------------------------------------------------------------------------- +// parse_count() +// ----------------------------------------------------------------------------- + +echo "parse_count()\n"; +$assert( 0, WorktreeStalenessProbe::parse_count( '' ), 'empty string → 0' ); +$assert( 0, WorktreeStalenessProbe::parse_count( "\n" ), 'newline only → 0' ); +$assert( 0, WorktreeStalenessProbe::parse_count( " \t\n" ), 'whitespace only → 0' ); +$assert( 3, WorktreeStalenessProbe::parse_count( '3' ), 'plain "3"' ); +$assert( 42, WorktreeStalenessProbe::parse_count( "42\n" ), 'trailing newline' ); +$assert( 7, WorktreeStalenessProbe::parse_count( " 7 " ), 'surrounding whitespace' ); +$assert( 0, WorktreeStalenessProbe::parse_count( 'abc' ), 'non-numeric → 0' ); +$assert( 0, WorktreeStalenessProbe::parse_count( '3 extra' ), 'mixed tokens → 0' ); + +// ----------------------------------------------------------------------------- +// is_missing_upstream() +// ----------------------------------------------------------------------------- + +echo "\nis_missing_upstream()\n"; +$assert( true, WorktreeStalenessProbe::is_missing_upstream( "fatal: no upstream configured for branch 'foo'" ), 'no upstream configured' ); +$assert( true, WorktreeStalenessProbe::is_missing_upstream( "fatal: ambiguous argument '@{upstream}': unknown revision" ), 'unknown revision' ); +$assert( true, WorktreeStalenessProbe::is_missing_upstream( "fatal: bad revision 'main..@{u}'" ), 'bad revision' ); +$assert( true, WorktreeStalenessProbe::is_missing_upstream( "fatal: Needed a single revision" ), 'needed a single revision' ); +$assert( false, WorktreeStalenessProbe::is_missing_upstream( "fatal: not a git repository" ), 'generic failure → false' ); +$assert( false, WorktreeStalenessProbe::is_missing_upstream( '' ), 'empty string → false' ); + +// ----------------------------------------------------------------------------- +// fetch() + behind_count() against real git fixtures +// ----------------------------------------------------------------------------- + +echo "\nfetch() + behind_count() against real git fixtures\n"; + +$which_git = trim( (string) shell_exec( 'command -v git 2>/dev/null' ) ); +if ( '' === $which_git ) { + echo " - skipped: git binary not available on PATH\n"; +} else { + // Build a tiny upstream repo with 3 commits on main. + $upstream_repo = sys_get_temp_dir() . '/dmc-stale-upstream-' . bin2hex( random_bytes( 4 ) ); + mkdir( $upstream_repo, 0755, true ); + exec( sprintf( 'cd %s && git init -q --initial-branch=main', escapeshellarg( $upstream_repo ) ) ); + for ( $i = 1; $i <= 3; $i++ ) { + file_put_contents( $upstream_repo . '/f.txt', "v{$i}\n" ); + exec( sprintf( + 'cd %s && git -c user.email=t@t -c user.name=t add -A && git -c user.email=t@t -c user.name=t commit -q -m "c%d"', + escapeshellarg( $upstream_repo ), + $i + ) ); + } + + // Clone into a consumer repo, then reset it back 2 commits so it is + // demonstrably "behind" origin/main. + $consumer = sys_get_temp_dir() . '/dmc-stale-consumer-' . bin2hex( random_bytes( 4 ) ); + exec( sprintf( 'git clone -q %s %s', escapeshellarg( $upstream_repo ), escapeshellarg( $consumer ) ) ); + exec( sprintf( 'cd %s && git reset -q --hard HEAD~2', escapeshellarg( $consumer ) ) ); + + // fetch() against a working `origin` → ok. + $ok = WorktreeStalenessProbe::fetch( $consumer ); + $assert( true, $ok['ok'], 'fetch() ok against real origin' ); + $assert_true( ! isset( $ok['error'] ), 'fetch() ok: no error field set' ); + + // behind_count() with upstream configured → 2. + $behind = WorktreeStalenessProbe::behind_count( $consumer, 'main', '@{upstream}' ); + $assert( 2, $behind, 'behind_count main..@{upstream} = 2 (reset back 2)' ); + + // behind_count() on a fresh local branch with no tracking → null. + exec( sprintf( 'cd %s && git checkout -q -b orphan', escapeshellarg( $consumer ) ) ); + $no_upstream = WorktreeStalenessProbe::behind_count( $consumer, 'orphan', '@{upstream}' ); + $assert( null, $no_upstream, 'behind_count() with no @{upstream} → null (not 0, not WP_Error)' ); + + // behind_count() against a non-existent ref → null (matches is_missing_upstream heuristic). + $bogus = WorktreeStalenessProbe::behind_count( $consumer, 'main', 'origin/does-not-exist' ); + $assert( null, $bogus, 'behind_count() with unknown upstream ref → null' ); + + // behind_count() where ref is up to date with upstream → 0, not null. + exec( sprintf( 'cd %s && git checkout -q main && git reset -q --hard origin/main', escapeshellarg( $consumer ) ) ); + $tip = WorktreeStalenessProbe::behind_count( $consumer, 'main', '@{upstream}' ); + $assert( 0, $tip, 'behind_count() at tip → 0 (distinct from null)' ); + + $cleanup( $consumer ); + $cleanup( $upstream_repo ); + + // fetch() against a repo with NO `origin` remote → ok=false + error populated. + $no_origin = sys_get_temp_dir() . '/dmc-stale-no-origin-' . bin2hex( random_bytes( 4 ) ); + mkdir( $no_origin, 0755, true ); + exec( sprintf( 'cd %s && git init -q --initial-branch=main', escapeshellarg( $no_origin ) ) ); + file_put_contents( $no_origin . '/x.txt', "x\n" ); + exec( sprintf( + 'cd %s && git -c user.email=t@t -c user.name=t add -A && git -c user.email=t@t -c user.name=t commit -q -m init', + escapeshellarg( $no_origin ) + ) ); + + $failed = WorktreeStalenessProbe::fetch( $no_origin ); + $assert( false, $failed['ok'], 'fetch() ok=false when no origin remote' ); + $assert_true( isset( $failed['error'] ) && '' !== $failed['error'], 'fetch() failure: error field populated' ); + // The error message should mention origin or remote or repository. + $assert_true( + isset( $failed['error'] ) && ( + false !== stripos( $failed['error'], 'origin' ) + || false !== stripos( $failed['error'], 'remote' ) + || false !== stripos( $failed['error'], 'repository' ) + ), + 'fetch() failure: error mentions origin/remote/repository' + ); + $cleanup( $no_origin ); +} + +// ----------------------------------------------------------------------------- +echo "\n{$total} total, "; +echo ( 0 === $failures ) ? "{$total}/{$total} passed\n" : "{$failures} failed\n"; +exit( $failures > 0 ? 1 : 0 );