refactor(space): consolidate rate/usage-limited paused-status predicate

[lsm]

Follow-up to #2271. Extracts the duplicated status === 'rate_limited' || status === 'usage_limited' check into a single isRateOrUsageLimited() predicate in shared/types/space-utils.ts and routes all ~13 call sites through it (processRunTick guard, concurrency-slot count, validateTaskAllowsSpawn, isLimited/restoreTaskFromRateLimit/parentLimited, isWorkflowRecoveryTransition, isActionRequired, isActiveTaskStatus, dependency-cancel, stopActiveWork, auto-clear-restrictions, shouldStopWorkflowForStatus). The paused-status set now lives in one place; semantics are unchanged and the existing rate-limit tests still pass.

Heads up: bun run check reports one check:test-quality failure in packages/daemon/.../provider-registry.test.ts that already exists on origin/dev and is unrelated to this change.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[greptile-apps]

Greptile Summary

This PR extracts the repeated status === 'rate_limited' || status === 'usage_limited' check into a single isRateOrUsageLimited() type predicate in packages/shared/src/types/space-utils.ts and threads all ~13 call sites through it. Semantics are unchanged; the diff is purely mechanical substitution.

• New predicate: isRateOrUsageLimited(status: SpaceTaskStatus): status is 'rate_limited' | 'usage_limited' is added to the shared utilities package with a JSDoc note explaining its role as the single source of truth.
• Call sites updated across seven daemon files and one web file, covering process-tick guards, concurrency-slot counts, dependency cancellation, workflow-recovery transitions, and the action-required UI predicate.
• Tests added: packages/shared/tests/space-utils.test.ts gains an exhaustiveness-checked ALL_SPACE_TASK_STATUSES array and two tests that verify both the predicate itself and that isWorkflowRecoveryTransition agrees with it across every known status.

Confidence Score: 5/5

Safe to merge — pure mechanical substitution with no logic changes across all 11 files.

Every changed line replaces a duplicated inline expression with a call to the new isRateOrUsageLimited() predicate that evaluates identically. The one slightly non-trivial conversion — the parentLimited ternary in task-agent-manager.ts — is correct: the original optional-chaining expression already returned false when parentTask was null, which the explicit ternary makes unambiguous. New tests cover the predicate exhaustively with a compile-time completeness guard. No behavioral delta found.

Files Needing Attention: No files require special attention.

Important Files Changed

Filename	Overview
packages/shared/src/types/space-utils.ts	Introduces isRateOrUsageLimited() type predicate with clear JSDoc; correctly declared as a type guard. isWorkflowRecoveryTransition is updated to call the predicate, consistent with all other consumers.
packages/shared/tests/space-utils.test.ts	Adds exhaustiveness-checked ALL_SPACE_TASK_STATUSES array with compile-time assertion pattern and two runtime tests verifying the predicate's return value set and its agreement with isWorkflowRecoveryTransition.
packages/daemon/src/lib/space/runtime/task-agent-manager.ts	Three call sites replaced; parentLimited ternary correctly preserves the prior null-safe false result when parentTask is absent.
packages/daemon/src/lib/space/runtime/space-runtime.ts	Two call sites updated (processRunTick guard, concurrency-slot count); both are mechanical substitutions with no logic change.
packages/daemon/src/lib/rpc-handlers/space-task-handlers.ts	Two call sites updated (fromActivePaused, toBlockedFromPaused); stopWorkflowForStatus logic unchanged.
packages/web/src/lib/task-filters.ts	isActionRequired updated to use the predicate for the rate/usage arm; blocked and review checks unchanged.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[SpaceTaskStatus] --> B{isRateOrUsageLimited?}
    B -->|'rate_limited' or 'usage_limited'| C[true — paused-on-cap]
    B -->|any other status| D[false]

    C --> E[processRunTick — skip tick]
    C --> F[concurrency slot count — holds slot]
    C --> G[validateTaskAllowsSpawn — transient block]
    C --> H[isActiveTaskStatus — goal stays active]
    C --> I[dependency cancel — cancel dependents]
    C --> J[isActionRequired — UI shows action needed]
    C --> K[isWorkflowRecoveryTransition — from paused → in_progress]
    C --> L[stopActiveWork — include in sweep]
    C --> M[auto-clear-restrictions — skip clear]
    C --> N[parentLimited — defer user message]

Loading

_{Reviews (2): Last reviewed commit: "test(space): enforce ALL_SPACE_TASK_STAT..." | Re-trigger Greptile}

[lsm]

🤖 Review by glm-5.1 (Zhipu AI)

Model: glm-5.1 | Client: NeoKai | Provider: Zhipu AI (z.ai)

Recommendation: REQUEST_CHANGES — one P2 on the new test's exhaustiveness guard. The refactor itself is clean and correct.

Verification done

• Inspected the diff across all 11 files. Every status === 'rate_limited' || status === 'usage_limited' predicate is now routed through isRateOrUsageLimited(). A repo-wide grep confirms the only remaining literals are legitimately non-predicate: SQL IN (...) CHECK constraints + the listRateLimitedTasks query, the status-value merging logic in task-agent-manager's limit-event handler, the SpaceTaskStatus union + message-type defs, UI status metadata, message-type matching in parse-group-message, and the superset active-status array in goal-automation-execute.handler.
• Semantics checked at all 13 call sites — equivalent, including the three non-trivial transforms: De Morgan at restoreTaskFromRateLimit and auto-clear-restrictions, and the null-safe rewrite at parentLimited. The status is 'rate_limited' | 'usage_limited' type guard preserves the same narrowing the inline checks had.
• Tests green: shared space-utils (34), web task-filters (22), daemon space-task-repository + space-task-manager (202), task-agent-rate-limit-listener + space-runtime-tick-loop (56), space-goal-service + space-task-handlers (119). tsc --noEmit exit 0, oxlint clean on changed files, knip clean for changed symbols.
• The check:test-quality failure in provider-registry.test.ts is pre-existing: that file is byte-identical to origin/dev and no provider code is touched here. Unrelated — accurate call in the PR description.

P2 — the "all consumers agree" test isn't compile-time-exhaustive (the task's own verification criterion)

ALL_SPACE_TASK_STATUSES is typed SpaceTaskStatus[], so if a future status is added to the union the array silently omits it and the exhaustive .filter(isRateOrUsageLimited) assertion passes without ever exercising the new value. That is exactly the drift the task's verification section wants to prevent ("add a status to the set → all sites pick it up"). The existing Greptile suggestion (as const satisfies readonly SpaceTaskStatus[]) catches typos/removed statuses but still does not enforce exhaustiveness — a partial array stays assignable to readonly SpaceTaskStatus[]. Recommend the literal tuple plus a bidirectional exhaustiveness assertion so a missing union member fails to compile. See the anchored comment for the exact pattern.

Once that lands this is an approve — the functional change is clean and the rate-limit semantics are unchanged.

[greptile-apps]

Want your agent to iterate on Greptile's feedback? Try greploops.

[lsm]

🤖 Review by glm-5.1 (Zhipu AI)

Model: glm-5.1 | Client: NeoKai | Provider: Zhipu AI (z.ai)

Recommendation: APPROVE — zero findings. The P2 from the previous round is genuinely resolved; my earlier skepticism was based on a flawed local reproduction (I had invoked plain tsc --noEmit at the project-reference root, which with files: [] checks nothing — my apologies for the noise).

Re-verified the exhaustiveness guard this round with the correct scoped typecheck (tsc -p packages/shared/tsconfig.json, which the real bun run check uses):

• Complete array → typecheck passes.
• Drop usage_limited → fails TS2322: Type 'true' is not assignable to type 'never' at the assertion line. ✅
• Drop done → same failure. ✅

(Distribution doesn't apply here — the conditional's left side is a concrete union, not a generic type parameter, so the never branch correctly survives and enforces completeness.)

Overall

• Refactor: all 13 predicate sites consolidated; repo-wide grep confirms the only remaining 'rate_limited'/'usage_limited' literals are legitimately non-predicate (SQL CHECK constraints + the listRateLimitedTasks query, status-value merging in the limit-event handler, the SpaceTaskStatus union + message-type defs, UI status metadata, message-type matching in parse-group-message, and the superset active-status array in goal-automation-execute.handler).
• Semantics equivalent at every site, including the De Morgan rewrites (restoreTaskFromRateLimit, auto-clear-restrictions) and the null-safe parentLimited ternary; the status is 'rate_limited' | 'usage_limited' type guard preserves the prior narrowing.
• bun run check: lint, typecheck, knip, session-guards, space-task-handler-tests, db-schema-parity all pass. The sole failure is check:test-quality on provider-registry.test.ts, which is byte-identical to origin/dev and unrelated (pre-existing).
• Affected tests green: shared space-utils (34), web task-filters (22), daemon repo+manager (202), rate-limit listener + tick-loop (56), goal-service + space-task-handlers (119).
• PR is OPEN, MERGEABLE, all review threads resolved.

Clean, well-scoped refactor. Approving.