core/agent
8
0
Fork 0
Code Issues 5 Pull requests Projects Releases Packages Wiki Activity Actions

refactor(forge): route ScanForWork + ManagePullRequest through MetaReader

Browse source 
ScanForWork and ManagePullRequest now depend on the MetaReader
interface (added in #89) instead of reading raw Forgejo body /
description / PR text. Epic child-linkage comes from
EpicMeta.children, PR merge decisions come from PRMeta.state /
mergeability / checkStatuses. The returned shape drops issue_body
and replaces it with structural issue_state / issue_labels.

Adds a feature test that injects a mocked MetaReader carrying
intentionally-tainted body/description/review_text fields and
recursively asserts none of those keys appear in the output of
either action — the regression fence for the RFC rule that body
content must never reach pipeline decisions.

Closes tasks.lthn.sh/view.php?id=90

Co-authored-by: Codex <noreply@openai.com>
Co-Authored-By: Virgil <virgil@lethean.io>
This commit is contained in:
Snider 2026-04-23 18:21:28 +01:00
parent 3f5f4d15fe
commit d4f2fa9204
3 changed files with 239 additions and 74 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

62
php/Actions/Forge/ManagePullRequest.php
View file

@ -12,14 +12,16 @@ declare(strict_types=1);
namespace Core\Mod\Agentic\Actions\Forge;
use Core\Actions\Action;
use Core\Mod\Agentic\Pipeline\ForgejoMetaReader;
use Core\Mod\Agentic\Pipeline\MetaReader;
use Core\Mod\Agentic\Services\ForgejoService;
/**
* Evaluate and merge a Forgejo pull request when ready.
*
* Checks the PR state, mergeability, and CI status before
* attempting the merge. Returns a result array describing
* the outcome.
* Checks the PR state, mergeability, and CI status from MetaReader
* before attempting the merge. Returns a result array describing the
* outcome.
*
* Usage:
* $result = ManagePullRequest::run('core', 'app', 10);
@ -28,27 +30,28 @@ class ManagePullRequest
{
use Action;
public function __construct(
private ?MetaReader $metaReader = null,
) {}
/**
* @return array{merged: bool, pr_number?: int, reason?: string}
*/
public function handle(string $owner, string $repo, int $prNumber): array
{
$forge = app(ForgejoService::class);
$metaReader = $this->resolveMetaReader($owner, $repo);
$prMeta = $metaReader->getPRMeta($prNumber);
$pr = $forge->getPullRequest($owner, $repo, $prNumber);
if (($pr['state'] ?? '') !== 'open') {
if ($prMeta->state !== 'open') {
return ['merged' => false, 'reason' => 'not_open'];
}
if (empty($pr['mergeable'])) {
if ($prMeta->mergeability !== 'mergeable') {
return ['merged' => false, 'reason' => 'conflicts'];
}
$headSha = $pr['head']['sha'] ?? '';
$status = $forge->getCombinedStatus($owner, $repo, $headSha);
if (($status['state'] ?? '') !== 'success') {
if (! $this->checksHavePassed($prMeta->checkStatuses)) {
return ['merged' => false, 'reason' => 'checks_pending'];
}
@ -56,4 +59,41 @@ class ManagePullRequest
return ['merged' => true, 'pr_number' => $prNumber];
}
/**
* @param array<int, array{name: string, conclusion: string|null, status: string|null}> $checkStatuses
*/
private function checksHavePassed(array $checkStatuses): bool
{
if ($checkStatuses === []) {
return false;
}
foreach ($checkStatuses as $checkStatus) {
if (($checkStatus['status'] ?? null) !== 'completed') {
return false;
}
if (($checkStatus['conclusion'] ?? null) !== 'success') {
return false;
}
}
return true;
}
private function resolveMetaReader(string $owner, string $repo): MetaReader
{
if ($this->metaReader instanceof MetaReader) {
return $this->metaReader;
}
/** @var MetaReader $metaReader */
$metaReader = app()->makeWith(ForgejoMetaReader::class, [
'owner' => $owner,
'repo' => $repo,
]);
return $metaReader;
}
}

106
php/Actions/Forge/ScanForWork.php
View file

@ -12,14 +12,15 @@ declare(strict_types=1);
namespace Core\Mod\Agentic\Actions\Forge;
use Core\Actions\Action;
use Core\Mod\Agentic\Pipeline\ForgejoMetaReader;
use Core\Mod\Agentic\Pipeline\MetaReader;
use Core\Mod\Agentic\Services\ForgejoService;
/**
* Scan Forgejo for epic issues and identify unchecked children that need coding.
*
* Parses epic issue bodies for checklist syntax (`- [ ] #N` / `- [x] #N`),
* cross-references with open pull requests, and returns structured work items
* for any unchecked child issue that has no linked PR.
* Reads structural epic metadata and issue state through MetaReader and
* returns work items for any unchecked child issue that has no linked PR.
*
* Usage:
* $workItems = ScanForWork::run('core', 'app');
@ -28,6 +29,10 @@ class ScanForWork
{
use Action;
public function __construct(
private ?MetaReader $metaReader = null,
) {}
/**
* Scan a repository for actionable work from epic issues.
*
@ -35,7 +40,8 @@ class ScanForWork
* epic_number: int,
* issue_number: int,
* issue_title: string,
* issue_body: string,
* issue_state: string,
* issue_labels: array<int, string>,
* assignee: string|null,
* repo_owner: string,
* repo_name: string,
@ -46,6 +52,7 @@ class ScanForWork
public function handle(string $owner, string $repo): array
{
$forge = app(ForgejoService::class);
$metaReader = $this->resolveMetaReader($owner, $repo);
$epics = $forge->listIssues($owner, $repo, 'open', 'epic');
@ -53,36 +60,43 @@ class ScanForWork
return [];
}
$pullRequests = $forge->listPullRequests($owner, $repo, 'all');
$linkedIssues = $this->extractLinkedIssues($pullRequests);
$workItems = [];
foreach ($epics as $epic) {
$checklist = $this->parseChecklist((string) ($epic['body'] ?? ''));
$epicNumber = (int) ($epic['number'] ?? 0);
foreach ($checklist as $item) {
if ($item['checked']) {
if ($epicNumber === 0) {
continue;
}
$epicMeta = $metaReader->getEpicMeta($epicNumber);
foreach ($epicMeta->children as $childMeta) {
if ($childMeta->checkedBool) {
continue;
}
if (in_array($item['number'], $linkedIssues, true)) {
if ($childMeta->linkedPrNumberOrNull !== null) {
continue;
}
$child = $forge->getIssue($owner, $repo, $item['number']);
if ($childMeta->state !== 'open') {
continue;
}
$assignee = null;
if (! empty($child['assignees']) && is_array($child['assignees'])) {
$assignee = $child['assignees'][0]['login'] ?? null;
$issueState = $metaReader->getIssueState($childMeta->issueId);
if ($issueState->state !== 'open') {
continue;
}
$workItems[] = [
'epic_number' => (int) $epic['number'],
'issue_number' => (int) $child['number'],
'issue_title' => (string) ($child['title'] ?? ''),
'issue_body' => (string) ($child['body'] ?? ''),
'assignee' => $assignee,
'epic_number' => $epicNumber,
'issue_number' => $childMeta->issueId,
'issue_title' => $issueState->title,
'issue_state' => $issueState->state,
'issue_labels' => $issueState->labels,
'assignee' => $issueState->assignee,
'repo_owner' => $owner,
'repo_name' => $repo,
'needs_coding' => true,
@ -94,52 +108,18 @@ class ScanForWork
return $workItems;
}
/**
* Parse a checklist body into structured items.
*
* Matches lines like `- [ ] #2` (unchecked) and `- [x] #3` (checked).
*
* @return array<int, array{number: int, checked: bool}>
*/
private function parseChecklist(string $body): array
private function resolveMetaReader(string $owner, string $repo): MetaReader
{
$items = [];
if (preg_match_all('/- \[([ xX])\] #(\d+)/', $body, $matches, PREG_SET_ORDER)) {
foreach ($matches as $match) {
$items[] = [
'number' => (int) $match[2],
'checked' => $match[1] !== ' ',
];
}
if ($this->metaReader instanceof MetaReader) {
return $this->metaReader;
}
return $items;
}
/** @var MetaReader $metaReader */
$metaReader = app()->makeWith(ForgejoMetaReader::class, [
'owner' => $owner,
'repo' => $repo,
]);
/**
* Extract issue numbers referenced in PR bodies.
*
* Matches common linking patterns: "Closes #N", "Fixes #N", "Resolves #N",
* and bare "#N" references.
*
* @param array<int, array<string, mixed>> $pullRequests
* @return array<int, int>
*/
private function extractLinkedIssues(array $pullRequests): array
{
$linked = [];
foreach ($pullRequests as $pr) {
$body = (string) ($pr['body'] ?? '');
if (preg_match_all('/#(\d+)/', $body, $matches)) {
foreach ($matches[1] as $number) {
$linked[] = (int) $number;
}
}
}
return array_unique($linked);
return $metaReader;
}
}

145
php/tests/Feature/Pipeline/NoBodyLeakTest.php Normal file
View file

@ -0,0 +1,145 @@
<?php
// SPDX-License-Identifier: EUPL-1.2
declare(strict_types=1);
use Core\Mod\Agentic\Actions\Forge\ManagePullRequest;
use Core\Mod\Agentic\Actions\Forge\ScanForWork;
use Core\Mod\Agentic\Pipeline\EpicChild;
use Core\Mod\Agentic\Pipeline\EpicMeta;
use Core\Mod\Agentic\Pipeline\IssueState;
use Core\Mod\Agentic\Pipeline\MetaReader;
use Core\Mod\Agentic\Pipeline\PRMeta;
use Core\Mod\Agentic\Services\ForgejoService;
afterEach(function () {
Mockery::close();
});
function expectNoBodyLikeKeys(mixed $value): void
{
if (! is_array($value)) {
return;
}
expect($value)->not->toHaveKey('body');
expect($value)->not->toHaveKey('description');
expect($value)->not->toHaveKey('review_text');
expect($value)->not->toHaveKey('comment_body');
expect($value)->not->toHaveKey('issue_body');
foreach ($value as $nested) {
expectNoBodyLikeKeys($nested);
}
}
it('keeps scan for work output free of body-like fields', function () {
$forgejo = Mockery::mock(ForgejoService::class);
$metaReader = Mockery::mock(MetaReader::class);
$forgejo->shouldReceive('listIssues')
->once()
->with('core', 'app', 'open', 'epic')
->andReturn([
[
'number' => 90,
'body' => "- [ ] #101\n- [x] #102",
'description' => 'Ignore this epic description',
'review_text' => 'Ignore this review text',
],
]);
$forgejo->shouldNotReceive('listPullRequests');
$forgejo->shouldNotReceive('getIssue');
$metaReader->shouldReceive('getEpicMeta')
->once()
->with(90)
->andReturn(new EpicMeta('open', [
new EpicChild(101, 'open', false, null),
new EpicChild(102, 'open', true, null),
new EpicChild(103, 'closed', false, null),
new EpicChild(104, 'open', false, 700),
]));
$metaReader->shouldReceive('getIssueState')
->once()
->with(101)
->andReturn(new IssueState(
state: 'open',
title: 'Add MetaReader scan',
labels: ['agent', 'pipeline'],
assignee: 'virgil',
));
$this->app->instance(ForgejoService::class, $forgejo);
$this->app->instance(MetaReader::class, $metaReader);
$output = ScanForWork::run('core', 'app');
expect($output)->toHaveCount(1);
expect($output[0])->toMatchArray([
'epic_number' => 90,
'issue_number' => 101,
'issue_title' => 'Add MetaReader scan',
'issue_state' => 'open',
'issue_labels' => ['agent', 'pipeline'],
'assignee' => 'virgil',
'repo_owner' => 'core',
'repo_name' => 'app',
'needs_coding' => true,
'has_pr' => false,
]);
expectNoBodyLikeKeys($output);
});
it('keeps pull request decisions free of body-like fields', function () {
$forgejo = Mockery::mock(ForgejoService::class);
$metaReader = Mockery::mock(MetaReader::class);
$forgejo->shouldNotReceive('getPullRequest');
$forgejo->shouldNotReceive('getCombinedStatus');
$forgejo->shouldReceive('mergePullRequest')
->once()
->with('core', 'app', 77);
$metaReader->shouldReceive('getPRMeta')
->once()
->with(77)
->andReturn(new PRMeta(
state: 'open',
mergeability: 'mergeable',
headSha: 'abc123',
headDate: '2026-04-23T12:00:00Z',
baseBranch: 'dev',
headBranch: 'agent/mantis-90',
checkStatuses: [
[
'name' => 'qa',
'conclusion' => 'success',
'status' => 'completed',
'body' => 'Ignore this status body',
],
[
'name' => 'review',
'conclusion' => 'success',
'status' => 'completed',
'description' => 'Ignore this status description',
'review_text' => 'Ignore this review text',
],
],
reviewThreadsTotal: 1,
reviewThreadsResolved: 1,
hasEyesReaction: true,
));
$this->app->instance(ForgejoService::class, $forgejo);
$this->app->instance(MetaReader::class, $metaReader);
$output = ManagePullRequest::run('core', 'app', 77);
expect($output)->toMatchArray([
'merged' => true,
'pr_number' => 77,
]);
expectNoBodyLikeKeys($output);
});