Skip to content

fix(bundle): mirror rebased handler throttles #1842

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
chubes4 merged 2 commits into from
May 7, 2026
Merged

fix(bundle): mirror rebased handler throttles #1842

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ed3de11
fix(bundle): mirror rebased handler throttles
chubes4 May 7, 2026
27d0fed
chore(bundle): align rebased throttle assignment
chubes4 May 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 95 additions & 4 deletions inc/Engine/Bundle/AgentBundleArtifactRebase.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,16 @@ public static function policy_burn_in_safe( array $input ): array {
$merged_step['handler_configs'] = $merged_hcs;
}

self::mirror_preserved_handler_throttle(
$base_hc,
$local_hc,
$base_hcs,
$local_hcs,
$step_id,
$merged_step,
$decisions
);

$merged_steps[ $step_id ] = $merged_step;
}

Expand All @@ -314,8 +324,8 @@ public static function policy_burn_in_safe( array $input ): array {
// it ambiguous so an operator confirms.
$top_keys = array_unique(
array_merge(
array_keys( is_array( $local ) ? $local : array() ),
array_keys( is_array( $remote ) ? $remote : array() )
array_keys( $local ),
array_keys( $remote )
)
);
foreach ( $top_keys as $key ) {
Expand Down Expand Up @@ -366,6 +376,87 @@ public static function policy_burn_in_safe( array $input ): array {
);
}

/**
* Keep local burn-in max_items consistent across legacy and canonical shapes.
*
* Flow payloads may carry both `handler_config` and `handler_configs` while
* installs migrate between old and new representations. The two shapes are
* equivalent runtime inputs, so a locally lowered throttle must not be
* preserved in only one of them.
*
* @param array<string,mixed> $base_hc Single-handler base config.
* @param array<string,mixed> $local_hc Single-handler local config.
* @param array<string,mixed> $base_hcs Multi-handler base config.
* @param array<string,mixed> $local_hcs Multi-handler local config.
* @param string|int $step_id Flow step ID.
* @param array<string,mixed> $merged_step In/out merged step.
* @param array<string,array<string,string>> $decisions In/out decisions map.
*/
private static function mirror_preserved_handler_throttle(
array $base_hc,
array $local_hc,
array $base_hcs,
array $local_hcs,
string|int $step_id,
array &$merged_step,
array &$decisions
): void {
$single_throttle = self::local_preserved_max_items( $base_hc, $local_hc );
if ( null !== $single_throttle && is_array( $merged_step['handler_configs'] ?? null ) ) {
foreach ( $merged_step['handler_configs'] as $slug => &$handler_config ) {
if ( ! is_array( $handler_config ) ) {
continue;
}
$handler_config['max_items'] = $single_throttle;
$decisions[ "flow_config.{$step_id}.handler_configs.{$slug}.max_items" ] = array(
'source' => 'local',
'reason' => 'burn_in_preserve_throttle_shape_mirror',
);
}
unset( $handler_config );
}

if ( ! is_array( $merged_step['handler_config'] ?? null ) ) {
return;
}

foreach ( $local_hcs as $slug => $local_config ) {
if ( ! is_array( $local_config ) ) {
continue;
}
$base_config = is_array( $base_hcs[ $slug ] ?? null ) ? $base_hcs[ $slug ] : array();
$max_items = self::local_preserved_max_items( $base_config, $local_config );
if ( null === $max_items ) {
continue;
}
$merged_step['handler_config']['max_items'] = $max_items;
$decisions[ "flow_config.{$step_id}.handler_config.max_items" ] = array(
'source' => 'local',
'reason' => 'burn_in_preserve_throttle_shape_mirror',
);
return;
}
}

/**
* Return locally changed max_items, or null when local did not change it.
*
* @param array<string,mixed> $base Base handler config.
* @param array<string,mixed> $local Local handler config.
*/
private static function local_preserved_max_items( array $base, array $local ): mixed {
if ( ! array_key_exists( 'max_items', $local ) ) {
return null;
}

$base_has = array_key_exists( 'max_items', $base );
if ( $base_has && $local['max_items'] === $base['max_items'] ) {
return null;
}

return $local['max_items'];
}

/**
* 3-way merge a single handler_config map.
*
Expand Down Expand Up @@ -421,7 +512,7 @@ private static function merge_handler_config(
);
continue;
}
if ( $local_changed && $remote_changed ) {
if ( $local_changed ) {
// Both moved this throttle. Default to local (safer) but flag ambiguous.
$merged[ $key ] = $local_val;
$decisions[ $path ] = array(
Expand Down Expand Up @@ -464,7 +555,7 @@ private static function merge_handler_config(
continue;
}

if ( $local_changed && $remote_changed ) {
if ( $local_changed ) {
// Both moved a non-throttle key. Default merged value to local
// (don't silently clobber) and flag for approval.
$merged[ $key ] = $local_val;
Expand Down
61 changes: 57 additions & 4 deletions tests/agent-bundle-artifact-rebase-smoke.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,9 +313,62 @@ function rebase_assert_equals( string $label, $expected, $actual ): void {
rebase_assert( 'multi: no ambiguous fields', empty( $multi_result['ambiguous'] ) );

// ---------------------------------------------------------------------------
// [6] Hashes are recomputed for the merged payload.
// [6] Equivalent legacy/canonical handler config shapes keep throttle in sync.
// ---------------------------------------------------------------------------
echo "\n[6] Rebase output carries reproducible hashes\n";
echo "\n[6] handler_config max_items mirrors into handler_configs\n";
$shape_result = AgentBundleArtifactRebase::rebase(
array(
'artifact_type' => 'flow',
'artifact_id' => 'shape-transition',
'base' => array(
'flow_config' => array(
'1_step_1' => array(
'handler_config' => array(
'owner' => 'old',
'max_items' => 25,
),
),
),
),
'local' => array(
'flow_config' => array(
'1_step_1' => array(
'handler_config' => array(
'owner' => 'old',
'max_items' => 1,
),
),
),
),
'remote' => array(
'flow_config' => array(
'1_step_1' => array(
'handler_config' => array(
'owner' => 'Automattic',
'max_items' => 25,
),
'handler_configs' => array(
'github-a8c' => array(
'owner' => 'Automattic',
'max_items' => 25,
),
),
),
),
),
),
AgentBundleArtifactRebase::POLICY_BURN_IN_SAFE
);

$shape_step = $shape_result['merged']['flow_config']['1_step_1'] ?? array();
rebase_assert_equals( 'shape: single handler throttle from local', 1, $shape_step['handler_config']['max_items'] ?? null );
rebase_assert_equals( 'shape: multi handler throttle mirrors local', 1, $shape_step['handler_configs']['github-a8c']['max_items'] ?? null );
rebase_assert_equals( 'shape: multi handler source-shape remains remote', 'Automattic', $shape_step['handler_configs']['github-a8c']['owner'] ?? null );

// ---------------------------------------------------------------------------
// [7] Hashes are recomputed for the merged payload.
// ---------------------------------------------------------------------------
echo "\n[7] Rebase output carries reproducible hashes\n";
$hashed_result = AgentBundleArtifactRebase::rebase(
array(
'artifact_type' => 'flow',
Expand All @@ -332,9 +385,9 @@ function rebase_assert_equals( string $label, $expected, $actual ): void {
rebase_assert( 'base_hash is set', is_string( $hashed_result['base_hash'] ) );

// ---------------------------------------------------------------------------
// [7] Default policy is conservative when an unknown policy name is supplied.
// [8] Default policy is conservative when an unknown policy name is supplied.
// ---------------------------------------------------------------------------
echo "\n[7] Unknown policy falls back to conservative\n";
echo "\n[8] Unknown policy falls back to conservative\n";
$unknown_result = AgentBundleArtifactRebase::rebase(
array(
'artifact_type' => 'flow',
Expand Down
Loading