fix: reject zero cleanup report limits (#499)

Author: chubes4

inc/Abilities/WorkspaceAbilities.php

@@ -1794,7 +1794,7 @@ private function registerAbilities(): void {
 							),
 							'limit'        => array(
 								'type'        => 'integer',
-								'description' => 'Page size for bounded dry-run, direct apply, budgeted apply, or job-backed apply.',
+								'description' => 'Positive page size for bounded dry-run, direct apply, budgeted apply, or job-backed apply.',
 							),
 							'offset'       => array(
 								'type'        => 'integer',
@@ -1835,7 +1835,7 @@ private function registerAbilities(): void {
 						'properties' => array(
 							'limit'        => array(
 								'type'        => 'integer',
-								'description' => 'Maximum active_no_signal rows to inspect in this page. Defaults to 25.',
+								'description' => 'Positive maximum active_no_signal rows to inspect in this page. Defaults to 25.',
 							),
 							'offset'       => array(
 								'type'        => 'integer',
@@ -1878,7 +1878,7 @@ private function registerAbilities(): void {
 							),
 							'limit'   => array(
 								'type'        => 'integer',
-								'description' => 'Maximum active_no_signal rows to inspect in this page. Defaults to 25.',
+								'description' => 'Positive maximum active_no_signal rows to inspect in this page. Defaults to 25.',
 							),
 							'offset'  => array(
 								'type'        => 'integer',
@@ -1918,7 +1918,7 @@ private function registerAbilities(): void {
 							),
 							'limit'   => array(
 								'type'        => 'integer',
-								'description' => 'Maximum active_no_signal rows to inspect in this page. Defaults to 25.',
+								'description' => 'Positive maximum active_no_signal rows to inspect in this page. Defaults to 25.',
 							),
 							'offset'  => array(
 								'type'        => 'integer',
@@ -1966,7 +1966,7 @@ private function registerAbilities(): void {
 							),
 							'limit'         => array(
 								'type'        => 'integer',
-								'description' => 'Maximum worktrees to scan in a dry-run page. Defaults to ' . Workspace::ARTIFACT_CLEANUP_DEFAULT_LIMIT . '. Use 0 to disable the cap (still bounded by exhaustive=false unless you also pass exhaustive=true).',
+								'description' => 'Positive maximum worktrees to scan in a dry-run page. Defaults to ' . Workspace::ARTIFACT_CLEANUP_DEFAULT_LIMIT . '. Use exhaustive=true for the explicit unbounded full audit mode.',
 							),
 							'offset'        => array(
 								'type'        => 'integer',

inc/Cli/Commands/WorkspaceCommand.php

@@ -2145,9 +2145,10 @@ private function renderGitOperationResult( string $operation, array $result, arr
 	 * [--limit=<count>]
 	 * : For `cleanup --dry-run`, `cleanup-artifacts --dry-run`,
 	 *   `reconcile-metadata`, and `active-no-signal-report`,
-	 *   maximum worktrees to scan in this page. Artifact cleanup defaults to
-	 *   100; metadata reconciliation uses this only when pagination is requested.
-	 *   Use 0 plus `--exhaustive` for a full artifact audit.
+	 *   positive maximum worktrees to scan in this page. Artifact cleanup defaults
+	 *   to 100; metadata reconciliation uses this only when pagination is
+	 *   requested. Use `--exhaustive` instead of `--limit=0` for a full artifact
+	 *   audit.
 	 *
 	 * [--offset=<count>]
 	 * : For `cleanup --dry-run`, `cleanup-artifacts --dry-run`,
@@ -2163,7 +2164,8 @@ private function renderGitOperationResult( string $operation, array $result, arr
 	 *
 	 * [--exhaustive]
 	 * : For `cleanup-artifacts --dry-run`, scan every worktree AND run per-worktree
-	 *   git safety probes. Slow on huge workspaces; use sparingly.
+	 *   git safety probes. This is the explicit unbounded artifact audit mode;
+	 *   slow on huge workspaces, so use sparingly.
 	 *
 	 * [--safety-probes]
 	 * : For `cleanup-artifacts --dry-run`, run per-worktree git safety probes

inc/Workspace/Workspace.php

@@ -1349,8 +1349,11 @@ function () use ( $repo, $branch, $wt_path, $force ) {
 	 */
 	public function worktree_active_no_signal_report( array $opts = array() ): array|\WP_Error {
 		$started_at = microtime(true);
-		$limit      = array_key_exists('limit', $opts) ? max(1, (int) $opts['limit']) : 25;
+		$limit      = array_key_exists('limit', $opts) ? (int) $opts['limit'] : 25;
 		$offset     = array_key_exists('offset', $opts) ? max(0, (int) $opts['offset']) : 0;
+		if ( $limit <= 0 ) {
+			return new \WP_Error('invalid_active_no_signal_limit', 'Active/no-signal report --limit must be greater than 0.', array( 'status' => 400 ));
+		}
 		if ( isset($opts['until_budget']) && '' !== trim( (string) $opts['until_budget']) ) {
 			$budget_seconds = $this->parse_worktree_metadata_reconciliation_budget(trim( (string) $opts['until_budget']));
 			if ( is_wp_error($budget_seconds) ) {

inc/Workspace/WorkspaceArtifactCleanup.php

@@ -43,7 +43,14 @@ public function worktree_cleanup_artifacts( array $opts = array() ): array|\WP_E
 		$exhaustive = ! empty($opts['exhaustive']);
 		$limit      = isset($opts['limit']) ? (int) $opts['limit'] : self::ARTIFACT_CLEANUP_DEFAULT_LIMIT;
 		$offset     = isset($opts['offset']) ? max(0, (int) $opts['offset']) : 0;
-		// Allow callers to opt out of bounded mode entirely.
+		if ( $limit < 0 ) {
+			return new \WP_Error('invalid_artifact_cleanup_limit', 'Artifact cleanup --limit must be greater than 0. Use --exhaustive for an unbounded full artifact audit.', array( 'status' => 400 ));
+		}
+		if ( ! $exhaustive && $limit <= 0 ) {
+			return new \WP_Error('invalid_artifact_cleanup_limit', 'Artifact cleanup --limit must be greater than 0. Use --exhaustive for an unbounded full artifact audit.', array( 'status' => 400 ));
+		}
+		// Allow callers to opt out of bounded mode entirely only through the
+		// explicit exhaustive path, which also enables safety probes.
 		if ( $exhaustive ) {
 			$limit = 0;
 		}
@@ -191,7 +198,7 @@ public function worktree_cleanup_artifacts( array $opts = array() ): array|\WP_E
 	 * `next_offset` continuation when the scan is partial.
 	 *
 	 * @param  bool  $force Whether to allow dirty/unpushed worktrees.
-	 * @param  array $opts  Options: `limit` (0 = unbounded), `offset`,
+	 * @param  array $opts  Options: `limit` (0 = unbounded internal exhaustive mode), `offset`,
 	 *                      `only_handles` (array<string>|null), `safety_probes`.
 	 * @return array{candidates: array<int,array>, skipped: array<int,array>, pagination: ?array<string,mixed>}|\WP_Error
 	 */

tests/smoke-worktree-cleanup-artifacts.php

@@ -236,6 +236,18 @@ function apply_filters( string $hook_name, $value )  // phpcs:ignore Generic.Cod
     $assert('unpushed_commits', $skip_reasons['demo@unpushed'] ?? '', 'exhaustive unpushed worktree is protected');
     $assert('active_symlink_target', $skip_reasons['demo@active'] ?? '', 'exhaustive active plugin symlink target is protected');
 
+    $exhaustive_limit_zero = $workspace->worktree_cleanup_artifacts(array( 'dry_run' => true, 'limit' => 0, 'exhaustive' => true ));
+    $assert(false, is_wp_error($exhaustive_limit_zero), 'limit=0 is accepted only with exhaustive artifact cleanup');
+    $assert('exhaustive', $exhaustive_limit_zero['pagination']['mode'] ?? '', 'limit=0 plus exhaustive reports exhaustive mode');
+
+    $limit_zero = $workspace->worktree_cleanup_artifacts(array( 'dry_run' => true, 'limit' => 0 ));
+    $assert(true, is_wp_error($limit_zero), 'limit=0 without exhaustive is rejected');
+    $assert('invalid_artifact_cleanup_limit', $limit_zero->code ?? '', 'limit=0 rejection uses explicit artifact cleanup error');
+    $negative_limit = $workspace->worktree_cleanup_artifacts(array( 'dry_run' => true, 'limit' => -1 ));
+    $assert(true, is_wp_error($negative_limit), 'negative artifact cleanup limit is rejected');
+    $negative_exhaustive_limit = $workspace->worktree_cleanup_artifacts(array( 'dry_run' => true, 'limit' => -1, 'exhaustive' => true ));
+    $assert(true, is_wp_error($negative_exhaustive_limit), 'negative artifact cleanup limit is rejected even with exhaustive');
+
     // Pagination smoke: limit=1 returns a partial scan with continuation.
     $page_one = $workspace->worktree_cleanup_artifacts(array( 'dry_run' => true, 'limit' => 1, 'offset' => 0 ));
     $assert(false, is_wp_error($page_one), 'page-1 dry-run succeeds');

tests/smoke-worktree-cleanup-cli.php

@@ -653,6 +653,9 @@ public function execute( array $input ): array
     datamachine_code_cleanup_assert(str_contains($cleanup_doc_comment, '<plan|apply|run|status|resume|cancel|evidence>'), 'workspace cleanup synopsis exposes DB-backed and task-backed cleanup operations');
     datamachine_code_cleanup_assert(str_contains($cleanup_doc_comment, '[--dry-run]'), 'task-backed cleanup synopsis keeps synchronous dry-run review');
     datamachine_code_cleanup_assert(str_contains($cleanup_doc_comment, 'apply runs freeze eligible candidates'), 'workspace cleanup limit help clarifies artifact apply scoping');
+    datamachine_code_cleanup_assert(str_contains($doc_comment, 'positive maximum worktrees to scan'), 'worktree limit help requires positive page sizes');
+    datamachine_code_cleanup_assert(str_contains($doc_comment, 'Use `--exhaustive` instead of `--limit=0`'), 'worktree limit help points unbounded artifact scans to exhaustive mode');
+    datamachine_code_cleanup_assert(str_contains($doc_comment, 'explicit unbounded artifact audit mode'), 'worktree exhaustive help documents unbounded artifact audit mode');
     datamachine_code_cleanup_assert(str_contains($doc_comment, 'Daily cleanup path: DB-backed plan, then apply only those rows after revalidation'), 'worktree examples point daily cleanup to DB-backed run_id controller path');
     datamachine_code_cleanup_assert(str_contains($doc_comment, 'workspace cleanup plan --mode=retention'), 'worktree examples include DB-backed cleanup plan');
     datamachine_code_cleanup_assert(str_contains($doc_comment, 'workspace cleanup run --mode=retention'), 'worktree examples include task-backed cleanup run');
@@ -932,6 +935,12 @@ public function execute( array $input ): array
     datamachine_code_cleanup_assert(str_contains(WP_CLI::$successes[0] ?? '', 'low-level escape hatch'), 'cleanup-artifacts dry-run demotes apply-plan wording');
     datamachine_code_cleanup_assert(! str_contains(WP_CLI::$successes[0] ?? '', 'Save JSON'), 'cleanup-artifacts dry-run does not normalize saving plan files');
 
+    WP_CLI::$logs      = array();
+    WP_CLI::$successes = array();
+    $command->worktree(array( 'cleanup-artifacts' ), array( 'dry-run' => true, 'limit' => 0, 'exhaustive' => true, 'format' => 'json' ));
+    datamachine_code_cleanup_assert(0 === (int) ( $artifact_ability->last_input['limit'] ?? -1 ), 'cleanup-artifacts forwards limit=0 when exhaustive is explicit');
+    datamachine_code_cleanup_assert(true === ( $artifact_ability->last_input['exhaustive'] ?? false ), 'cleanup-artifacts forwards exhaustive flag');
+
     $artifact_plan_file = sys_get_temp_dir() . '/dmc-artifact-cleanup-plan-' . bin2hex(random_bytes(3)) . '.json';
     file_put_contents($artifact_plan_file, wp_json_encode($artifact_json));
     WP_CLI::$logs      = array();

tests/smoke-worktree-metadata-reconcile.php

@@ -558,6 +558,13 @@ function () use ( $tmp ) {
     $assert(true, ! is_wp_error($budgeted_active_report) && ( $budgeted_active_report['success'] ?? false ), 'budgeted active/no-signal report succeeds');
     $assert(true, (bool) ( $budgeted_active_report['pagination']['partial'] ?? false ), 'budgeted active/no-signal report returns partial pagination');
     $assert(true, isset($budgeted_active_report['evidence']['budget']['budget_exhausted']), 'budgeted active/no-signal report exposes budget evidence');
+    $bad_active_zero = $ws->worktree_active_no_signal_report(array( 'limit' => 0, 'offset' => 0 ));
+    $assert(true, is_wp_error($bad_active_zero), 'active/no-signal report rejects limit=0');
+    $assert('invalid_active_no_signal_limit', $bad_active_zero->code ?? '', 'active/no-signal limit=0 rejection uses explicit error code');
+    $bad_active_negative = $ws->worktree_active_no_signal_report(array( 'limit' => -1, 'offset' => 0 ));
+    $assert(true, is_wp_error($bad_active_negative), 'active/no-signal report rejects negative limits');
+    $bad_active_budget = $ws->worktree_active_no_signal_report(array( 'limit' => 0, 'until_budget' => '1s' ));
+    $assert(true, is_wp_error($bad_active_budget), 'budgeted active/no-signal report still requires a positive page size');
     $run('git remote set-url origin https://github.com/acme/demo.git', $primary);
     $finalized_dry_run = $ws->worktree_active_no_signal_finalized_apply(array( 'dry_run' => true, 'limit' => 50, 'offset' => 0 ));
     $run(sprintf('git remote set-url origin %s', escapeshellarg($remote)), $primary);
@@ -625,6 +632,14 @@ function () use ( $tmp ) {
     $assert(true, (bool) ( $budgeted_page['pagination']['partial'] ?? false ), 'budgeted metadata reconciliation dry-run returns partial pagination');
     $assert(true, isset($budgeted_page['evidence']['budget']['budget_exhausted']), 'budgeted metadata reconciliation dry-run exposes budget evidence');
 
+    $bad_reconcile_zero = $ws->worktree_reconcile_metadata(array( 'dry_run' => true, 'limit' => 0, 'offset' => 0 ));
+    $assert(true, is_wp_error($bad_reconcile_zero), 'metadata reconciliation rejects limit=0 when pagination is requested');
+    $assert('invalid_metadata_reconcile_limit', $bad_reconcile_zero->code ?? '', 'metadata reconciliation limit=0 rejection keeps explicit error code');
+    $bad_reconcile_negative = $ws->worktree_reconcile_metadata(array( 'dry_run' => true, 'limit' => -1, 'offset' => 0 ));
+    $assert(true, is_wp_error($bad_reconcile_negative), 'metadata reconciliation rejects negative limits');
+    $bad_reconcile_budget = $ws->worktree_reconcile_metadata(array( 'apply' => true, 'limit' => 0, 'until_budget' => '1s' ));
+    $assert(true, is_wp_error($bad_reconcile_budget), 'budgeted metadata reconciliation direct apply requires a positive page size');
+
     \DataMachine\Engine\Tasks\TaskScheduler::$batches = array();
     $job_backed = $ws->worktree_reconcile_metadata(array( 'apply' => true, 'via_jobs' => true, 'limit' => 3 ));
     $assert(true, ! is_wp_error($job_backed) && ( $job_backed['success'] ?? false ), 'job-backed reconciliation scheduling succeeds');