Skip to content

Commit b100b25

Browse files
chubes4homeboy-ci[bot]
chubes4
and
homeboy-ci[bot]
authored
refactor: isolate PR run artifact access (#770)
Co-authored-by: homeboy-ci[bot] <266378653+homeboy-ci[bot]@users.noreply.github.com>
1 parent c019fb4 commit b100b25

4 files changed

Lines changed: 179 additions & 37 deletions

File tree

‎inc/Handlers/GitHub/GitHubPullRequestPublish.php‎

Lines changed: 9 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010
use DataMachine\Core\Steps\HandlerRegistrationTrait;
1111
use DataMachine\Core\Steps\Publish\Handlers\PublishHandler;
1212
use DataMachineCode\Abilities\GitHubAbilities;
13+
use DataMachineCode\RunArtifacts\DataMachineRunArtifactRepository;
14+
use DataMachineCode\RunArtifacts\RunArtifactRepositoryInterface;
1315
use DataMachineCode\Support\RunArtifactBundleFileWriter;
1416
use DataMachineCode\Support\RunArtifactPrSectionRenderer;
1517

@@ -23,8 +25,11 @@ class GitHubPullRequestPublish extends PublishHandler {
2325

2426
use HandlerRegistrationTrait;
2527

26-
public function __construct() {
28+
private RunArtifactRepositoryInterface $run_artifact_repository;
29+
30+
public function __construct( ?RunArtifactRepositoryInterface $run_artifact_repository = null ) {
2731
parent::__construct('github_pull_request');
32+
$this->run_artifact_repository = $run_artifact_repository ?? new DataMachineRunArtifactRepository();
2833

2934
self::registerHandler(
3035
'github_pull_request',
@@ -305,7 +310,7 @@ private function prepareRunArtifactAttachment( array $parameters, array $handler
305310
return array();
306311
}
307312

308-
$artifacts = $this->jobArtifactsForPublish($job_id);
313+
$artifacts = $this->run_artifact_repository->artifacts_for_job($job_id);
309314
if ( empty($artifacts) ) {
310315
return array();
311316
}
@@ -331,7 +336,7 @@ private function prepareRunArtifactAttachment( array $parameters, array $handler
331336
* @return array<int,array<string,mixed>> File records accepted by the normal files loop.
332337
*/
333338
private function bundleFileArtifactsForPublish( array $parameters, array $handler_config ): array {
334-
$artifacts = is_array($parameters['run_artifacts'] ?? null) ? $parameters['run_artifacts'] : $this->jobArtifactsForPublish( (int) ( $parameters['job_id'] ?? 0 ));
339+
$artifacts = is_array($parameters['run_artifacts'] ?? null) ? $parameters['run_artifacts'] : $this->run_artifact_repository->artifacts_for_job( (int) ( $parameters['job_id'] ?? 0 ));
335340
if ( empty($artifacts) ) {
336341
return array();
337342
}
@@ -359,7 +364,7 @@ private function resolveRunArtifactPolicy( array $parameters ): array {
359364
}
360365
}
361366

362-
return $this->jobRunArtifactEgressPolicy( (int) ( $parameters['job_id'] ?? 0 ));
367+
return $this->run_artifact_repository->egress_policy_for_job( (int) ( $parameters['job_id'] ?? 0 ));
363368
}
364369

365370
/**
@@ -417,39 +422,6 @@ private function resolveRunArtifactAttachmentMode( array $handler_config ): stri
417422
return RunArtifactPrSectionRenderer::MODE_COMMENT === $mode ? RunArtifactPrSectionRenderer::MODE_COMMENT : RunArtifactPrSectionRenderer::MODE_BODY_SECTION;
418423
}
419424

420-
/**
421-
* @return array<string,mixed>
422-
*/
423-
private function jobArtifactsForPublish( int $job_id ): array {
424-
if ( $job_id <= 0 || ! class_exists('\\DataMachine\\Core\\JobArtifacts') ) {
425-
return array();
426-
}
427-
428-
$result = ( new \DataMachine\Core\JobArtifacts() )->get($job_id);
429-
if ( empty($result['success']) || ! is_array($result['artifacts'] ?? null) ) {
430-
return array();
431-
}
432-
433-
return $result['artifacts'];
434-
}
435-
436-
/**
437-
* @return array<string,mixed>
438-
*/
439-
private function jobRunArtifactEgressPolicy( int $job_id ): array {
440-
if ( $job_id <= 0 || ! class_exists('\\DataMachine\\Core\\Database\\Jobs\\Jobs') ) {
441-
return array();
442-
}
443-
444-
$jobs = new \DataMachine\Core\Database\Jobs\Jobs();
445-
if ( ! method_exists($jobs, 'retrieve_engine_data') ) {
446-
return array();
447-
}
448-
449-
$engine_data = $jobs->retrieve_engine_data($job_id);
450-
return is_array($engine_data['run_artifact_egress_policy'] ?? null) ? $engine_data['run_artifact_egress_policy'] : array();
451-
}
452-
453425
/**
454426
* Resolve a list supplied by tool parameters or comma-separated handler config.
455427
*

‎inc/RunArtifacts/DataMachineRunArtifactRepository.php‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
<?php
2+
/**
3+
* Data Machine-backed run artifact repository.
4+
*
5+
* @package DataMachineCode\RunArtifacts
6+
*/
7+
8+
namespace DataMachineCode\RunArtifacts;
9+
10+
defined('ABSPATH') || exit;
11+
12+
class DataMachineRunArtifactRepository implements RunArtifactRepositoryInterface {
13+
14+
15+
16+
/**
17+
* @return array<string,mixed>
18+
*/
19+
public function artifacts_for_job( int $job_id ): array {
20+
if ( $job_id <= 0 || ! class_exists('\\DataMachine\\Core\\JobArtifacts') ) {
21+
return array();
22+
}
23+
24+
$result = ( new \DataMachine\Core\JobArtifacts() )->get($job_id);
25+
if ( empty($result['success']) || ! is_array($result['artifacts'] ?? null) ) {
26+
return array();
27+
}
28+
29+
return $result['artifacts'];
30+
}
31+
32+
/**
33+
* @return array<string,array<string,mixed>>
34+
*/
35+
public function egress_policy_for_job( int $job_id ): array {
36+
if ( $job_id <= 0 || ! class_exists('\\DataMachine\\Core\\Database\\Jobs\\Jobs') ) {
37+
return array();
38+
}
39+
40+
$jobs = new \DataMachine\Core\Database\Jobs\Jobs();
41+
if ( ! method_exists($jobs, 'retrieve_engine_data') ) {
42+
return array();
43+
}
44+
45+
$engine_data = $jobs->retrieve_engine_data($job_id);
46+
return is_array($engine_data['run_artifact_egress_policy'] ?? null) ? $engine_data['run_artifact_egress_policy'] : array();
47+
}
48+
}

‎inc/RunArtifacts/RunArtifactRepositoryInterface.php‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<?php
2+
/**
3+
* Run artifact repository contract.
4+
*
5+
* @package DataMachineCode\RunArtifacts
6+
*/
7+
8+
namespace DataMachineCode\RunArtifacts;
9+
10+
defined('ABSPATH') || exit;
11+
12+
interface RunArtifactRepositoryInterface {
13+
14+
15+
16+
/**
17+
* Read the artifact payload for a completed runtime job.
18+
*
19+
* @param int $job_id Runtime job identifier.
20+
* @return array<string,mixed>
21+
*/
22+
public function artifacts_for_job( int $job_id ): array;
23+
24+
/**
25+
* Read the artifact egress policy captured with a runtime job.
26+
*
27+
* @param int $job_id Runtime job identifier.
28+
* @return array<string,array<string,mixed>>
29+
*/
30+
public function egress_policy_for_job( int $job_id ): array;
31+
}

‎tests/run-artifact-repository-boundary.php‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace DataMachine\Core\Steps {
6+
trait HandlerRegistrationTrait {
7+
public static function registerHandler( mixed ...$args ): void {}
8+
}
9+
}
10+
11+
namespace DataMachine\Core\Steps\Publish\Handlers {
12+
class PublishHandler {
13+
public function __construct( string $slug ) {}
14+
}
15+
}
16+
17+
namespace {
18+
if ( ! defined('ABSPATH') ) {
19+
define('ABSPATH', __DIR__ . '/fixtures/');
20+
}
21+
22+
if ( ! function_exists('sanitize_key') ) {
23+
function sanitize_key( string $key ): string {
24+
return strtolower(preg_replace('/[^a-zA-Z0-9_\-]/', '', $key) ?? '');
25+
}
26+
}
27+
28+
if ( ! function_exists('sanitize_title') ) {
29+
function sanitize_title( string $title ): string {
30+
$title = strtolower(trim($title));
31+
$title = preg_replace('/[^a-z0-9]+/', '-', $title) ?? '';
32+
return trim($title, '-');
33+
}
34+
}
35+
36+
require_once dirname(__DIR__) . '/inc/RunArtifacts/RunArtifactRepositoryInterface.php';
37+
require_once dirname(__DIR__) . '/inc/Support/RunArtifactBundleFileWriter.php';
38+
require_once dirname(__DIR__) . '/inc/Handlers/GitHub/GitHubPullRequestPublish.php';
39+
40+
use DataMachineCode\Handlers\GitHub\GitHubPullRequestPublish;
41+
use DataMachineCode\RunArtifacts\RunArtifactRepositoryInterface;
42+
43+
final class FakeRunArtifactRepository implements RunArtifactRepositoryInterface {
44+
public int $artifact_job_id = 0;
45+
public int $policy_job_id = 0;
46+
47+
public function artifacts_for_job( int $job_id ): array {
48+
$this->artifact_job_id = $job_id;
49+
50+
return array(
51+
'daily_memory_artifacts' => array(
52+
array(
53+
'type' => 'daily_memory',
54+
'agent_slug' => 'boundary agent',
55+
'bundle_relative_path' => 'memory.md',
56+
'content' => '# Boundary evidence',
57+
),
58+
),
59+
);
60+
}
61+
62+
public function egress_policy_for_job( int $job_id ): array {
63+
$this->policy_job_id = $job_id;
64+
65+
return array(
66+
'daily_memory' => array(
67+
'egress' => array( 'bundle-file' ),
68+
),
69+
);
70+
}
71+
}
72+
73+
function boundary_assert_same( mixed $expected, mixed $actual, string $message ): void {
74+
if ( $expected !== $actual ) {
75+
throw new RuntimeException(sprintf("%s\nExpected: %s\nActual: %s", $message, var_export($expected, true), var_export($actual, true)));
76+
}
77+
}
78+
79+
$repository = new FakeRunArtifactRepository();
80+
$handler = new GitHubPullRequestPublish($repository);
81+
$method = new ReflectionMethod($handler, 'bundleFileArtifactsForPublish');
82+
83+
$files = $method->invoke($handler, array( 'job_id' => 42 ), array( 'bundle_root' => 'bundles/{agent_slug}' ));
84+
85+
boundary_assert_same(42, $repository->artifact_job_id, 'Handler should read artifacts through the injected repository.');
86+
boundary_assert_same(42, $repository->policy_job_id, 'Handler should read egress policy through the injected repository.');
87+
boundary_assert_same('bundles/boundary-agent/memory.md', $files[0]['file_path'] ?? null, 'Injected repository artifacts should still become bundle-file writes.');
88+
boundary_assert_same('# Boundary evidence', $files[0]['content'] ?? null, 'Artifact content should be preserved.');
89+
90+
echo "run artifact repository boundary test passed.\n";
91+
}

0 commit comments

Comments
 (0)