Skip to content

Commit 78126a0

Browse files
Bound worktree freshness fetch (#929)
Co-authored-by: homeboy-ci[bot] <266378653+homeboy-ci[bot]@users.noreply.github.com>
1 parent 92a814c commit 78126a0

5 files changed

Lines changed: 94 additions & 4 deletions

File tree

inc/Abilities/WorkspaceAbilities.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1376,6 +1376,14 @@ private function registerAbilities(): void {
13761376
'type' => 'string',
13771377
'description' => 'Present only when fetch_failed=true. Trimmed error output from the failing fetch.',
13781378
),
1379+
'fetch_timed_out' => array(
1380+
'type' => 'boolean',
1381+
'description' => 'Present only when the pre-create freshness fetch timed out and allow_unverified_freshness=true allowed creation to continue. This is not stale-repository evidence.',
1382+
),
1383+
'fetch_timeout_seconds' => array(
1384+
'type' => 'integer',
1385+
'description' => 'Present only when fetch_timed_out=true. The bounded freshness-fetch budget in seconds.',
1386+
),
13791387
'stale_commits_behind' => array(
13801388
'type' => 'integer',
13811389
'description' => 'For the existing-local-branch path, how many commits the worktree branch is behind its configured upstream. Omitted when no upstream is configured.',

inc/Workspace/WorkspaceWorktreeLifecycle.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,8 @@ private function worktree_add_locked(
265265
$fetch = WorktreeStalenessProbe::fetch($primary_path);
266266
$fetch_failed = ! $fetch['ok'];
267267
$fetch_error = $fetch['error'] ?? null;
268+
$fetch_timed_out = ! empty($fetch['timed_out']);
269+
$fetch_timeout_seconds = $fetch['timeout_seconds'] ?? null;
268270
if ( $fetch_failed && ! $allow_unverified_freshness ) {
269271
return new \WP_Error(
270272
'worktree_freshness_unverified',
@@ -273,6 +275,8 @@ private function worktree_add_locked(
273275
'status' => 409,
274276
'fetch_failed' => true,
275277
'fetch_error' => $fetch_error,
278+
'fetch_timed_out' => $fetch_timed_out,
279+
'fetch_timeout_seconds' => $fetch_timeout_seconds,
276280
'allow_unverified_freshness' => false,
277281
)
278282
);
@@ -321,6 +325,10 @@ private function worktree_add_locked(
321325

322326
if ( $fetch_failed ) {
323327
$response['fetch_failed'] = true;
328+
if ( $fetch_timed_out ) {
329+
$response['fetch_timed_out'] = true;
330+
$response['fetch_timeout_seconds'] = $fetch_timeout_seconds;
331+
}
324332
if ( null !== $fetch_error && '' !== $fetch_error ) {
325333
$response['fetch_error'] = $fetch_error;
326334
}

inc/Workspace/WorktreeStalenessProbe.php

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,21 +32,38 @@
3232

3333
final class WorktreeStalenessProbe {
3434

35+
/**
36+
* Keep the worktree-creation freshness check aligned with bounded lifecycle
37+
* git probes, without allowing a remote to hold the workspace mutation lock.
38+
*/
39+
private const FETCH_TIMEOUT_SECONDS = 5;
3540

3641

3742
/**
3843
* Fetch `origin` in a repository. Returns structured result instead of
3944
* throwing so the caller can decide whether to continue on failure.
4045
*
41-
* @param string $repo_path Primary repo path (passed to `git -C`).
42-
* @return array{ok: bool, error?: string}
46+
* @param string $repo_path Primary repo path (passed to `git -C`).
47+
* @param callable|null $runner Optional git runner, used by deterministic tests.
48+
* @return array{ok: bool, error?: string, timed_out?: bool, timeout_seconds?: int}
4349
*/
44-
public static function fetch( string $repo_path ): array {
45-
$result = GitRunner::run($repo_path, 'fetch --quiet origin');
50+
public static function fetch( string $repo_path, ?callable $runner = null ): array {
51+
$runner = $runner ?? static fn( string $path, string $args, int $timeout ): array|\WP_Error => GitRunner::run($path, $args, $timeout);
52+
$result = $runner($repo_path, 'fetch --quiet origin', self::FETCH_TIMEOUT_SECONDS);
4653
if ( is_wp_error($result) ) {
4754
$data = $result->get_error_data();
4855
$tail = is_array($data) && isset($data['output']) ? trim( (string) $data['output']) : '';
4956
$error = '' !== $tail ? $tail : $result->get_error_message();
57+
$code = method_exists($result, 'get_error_code') ? $result->get_error_code() : '';
58+
if ( 'git_command_timeout' === $code ) {
59+
return array(
60+
'ok' => false,
61+
'timed_out' => true,
62+
'timeout_seconds' => self::FETCH_TIMEOUT_SECONDS,
63+
'error' => sprintf('Remote freshness fetch timed out after %d seconds. Check origin connectivity or credentials and retry; use allow_unverified_freshness=true only for intentional offline work.', self::FETCH_TIMEOUT_SECONDS),
64+
);
65+
}
66+
5067
return array(
5168
'ok' => false,
5269
'error' => $error,

tests/process-runner-command-spec.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,16 @@ function process_runner_assert_not_error( mixed $result, string $message ): void
7070
process_runner_assert_same('argv-ok|' . basename($cwd), $result['stdout'], 'CommandSpec preserves argv, cwd, and env policy.');
7171
process_runner_assert_same('', $result['stderr'], 'CommandSpec captures stderr separately.');
7272

73+
$timed_out = ProcessRunner::run(
74+
CommandSpec::from_argv(array( PHP_BINARY, '-r', 'while (true) {}' )),
75+
array(
76+
'timeout_seconds' => 1,
77+
'poll_interval_microseconds' => 1000,
78+
)
79+
);
80+
process_runner_assert_same(true, $timed_out instanceof WP_Error, 'ProcessRunner should terminate a controlled hanging process.');
81+
process_runner_assert_same(1, $timed_out->get_error_data()['timeout'] ?? null, 'ProcessRunner timeout result carries the configured budget.');
82+
7383
$invalid = CommandSpec::from_argv(array());
7484
process_runner_assert_same(true, $invalid instanceof WP_Error, 'CommandSpec rejects empty argv.');
7585

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
if ( ! defined('ABSPATH') ) {
6+
define('ABSPATH', __DIR__ . '/fixtures/');
7+
}
8+
9+
if ( ! class_exists('WP_Error') ) {
10+
final class WP_Error {
11+
public function __construct( private string $code = '', private string $message = '', private array $data = array() ) {}
12+
public function get_error_code(): string { return $this->code; }
13+
public function get_error_message(): string { return $this->message; }
14+
public function get_error_data(): array { return $this->data; }
15+
}
16+
}
17+
18+
function is_wp_error( mixed $value ): bool {
19+
return $value instanceof WP_Error;
20+
}
21+
22+
require_once dirname(__DIR__) . '/inc/Workspace/WorktreeStalenessProbe.php';
23+
24+
use DataMachineCode\Workspace\WorktreeStalenessProbe;
25+
26+
function staleness_timeout_assert_same( mixed $expected, mixed $actual, string $message ): void {
27+
if ( $expected !== $actual ) {
28+
throw new RuntimeException($message . ' Expected ' . var_export($expected, true) . ', got ' . var_export($actual, true) . '.');
29+
}
30+
}
31+
32+
$calls = array();
33+
$result = WorktreeStalenessProbe::fetch(
34+
'/repo',
35+
static function ( string $path, string $args, int $timeout ) use ( &$calls ): WP_Error {
36+
$calls[] = array( $path, $args, $timeout );
37+
return new WP_Error('git_command_timeout', 'Process command timed out after 5 second(s).', array( 'timeout' => 5 ));
38+
}
39+
);
40+
41+
staleness_timeout_assert_same(array( array( '/repo', 'fetch --quiet origin', 5 ) ), $calls, 'Freshness fetch must use the bounded GitRunner timeout contract.');
42+
staleness_timeout_assert_same(false, $result['ok'], 'Timed-out freshness fetch must fail verification.');
43+
staleness_timeout_assert_same(true, $result['timed_out'] ?? null, 'Timed-out freshness fetch must remain distinct from stale evidence.');
44+
staleness_timeout_assert_same(5, $result['timeout_seconds'] ?? null, 'Timed-out freshness fetch must surface its budget.');
45+
staleness_timeout_assert_same(true, str_contains((string) ( $result['error'] ?? '' ), 'allow_unverified_freshness=true'), 'Timed-out freshness fetch must provide an actionable opt-in diagnostic.');
46+
47+
fwrite(STDOUT, "worktree-staleness-fetch-timeout: ok\n");

0 commit comments

Comments
 (0)