Refactor update mutation complexity

Merged
UN
4mo ago
+1626-1208

Summary

Linked pm item: pm-0n6p under pm-92if.

This PR finishes the CodeFactor complexity remediation for the item-mutation path:

  • Decomposes runUpdate and its update mutation callback in src/cli/commands/update.ts into focused helpers while preserving update routing, close-status routing, dependency/test/doc/file mutations, runtime fields, and changed-field behavior.
  • Decomposes large mutation registration action closures in src/cli/register-mutation.ts into named action helpers.
  • Removes the target complexity suppressions from eslint-suppressions.json.
  • Adds register-command coverage for repeatable schema role/alias handling and defensive programmatic string coercion.
  • Records implementation, verification, and Greptile evidence on pm-0n6p through the pm CLI.

Verification

  • npm install -g .
  • pm --version -> 2026.6.30
  • node -v -> v26.4.0
  • pnpm -v -> 10.33.4
  • pnpm build
  • unsuppressed ESLint for changed code/test files
  • pnpm typecheck
  • node scripts/run-tests.mjs test -- tests/unit/commands/update-command.spec.ts tests/unit/commands/register-commands.spec.ts -> 175 tests passed
  • node scripts/run-tests.mjs coverage -> 262 files / 5265 tests, 100% statements / branches / functions / lines
  • linked pm test pm-0n6p --run --progress --pm-context tracker --override-linked-pm-context -> coverage, focused tests, contracts, lint all passed
  • pnpm contracts:check
  • pnpm lint -> ESLint, jscpd, static quality gate passed
  • pnpm changelog:pm:check -> latest pm-changelog generated no CHANGELOG.md change
  • pnpm exec node scripts/check-secrets.mjs -> no credential-like secrets in tracked files
  • pnpm audit --audit-level moderate -> no known vulnerabilities
  • pm health --check-only --summary --json -> ok
  • pm validate --check-resolution --check-history-drift --check-command-references --strict-exit --json -> ok with pre-existing resolution metadata warnings; command references and history drift clean
  • git diff --check

Manual Smoke

Installed-package smoke in an isolated temp project (/tmp/tmp.3bglbDFU7O):

  • pm init --defaults --author manual-smoke
  • pm schema add-status review --role active --role blocked --alias in_review --alias needs_review --order 20
  • pm schema add-type Spike --description "Manual smoke investigation" --folder spikes --default-status open --alias Investigation
  • pm schema add-field severity_level --type string --commands create,update,list --alias severity --required-types Spike
  • created and updated a Spike item using runtime schema flags, then verified pm get, pm list, and pm health

Final smoke assertion: custom Spike item reached review, severity_level=high, tags codefactor,sdk, list count 1, health ok.

GitHub / Release / Telemetry Sweep

  • No open PRs before this one.
  • No open GitHub issues.
  • No open Dependabot alerts.
  • No open code scanning alerts.
  • Latest scheduled Auto Release run checked: success on 2026-07-01T06:52:39Z for main b2617dac.
  • npm view @unbrained/pm-cli version returned 2026.7.1 during the sweep.
  • Sentry issue list reachable; sampled unresolved PM-CLI-16 and PM-CLI-1Q were stale with lastSeen 2026-06-13, not evidence of a current regression.
  • pm telemetry status --json showed telemetry enabled with recent successful flush on 2026-07-01T08:03:07.606Z.

Greptile

Pre-push Greptile review loop:

  • ee898fbd-4a0d-48f9-9fce-5b22c99f77c5 found dead sprintReleasePolicy; fixed.
  • 3df1cc5f-6934-4d13-b26e-4a26ea2dea8a found two behavior-preservation concerns; fixed defensive schema string coercion and replace-tests empty cleanup.
  • 4cd22b0a-0678-40ee-b5e5-f24b4863ced0 returned COMMENTS=0, confidence 4, safe to merge.

Summary by cubic

Refactors the update command and mutation registration into focused helpers to lower CodeFactor complexity and complete runtime‑aware option‑policy checks. No CLI contract changes; closes pm-0n6p (under pm-92if) and removes the last ESLint complexity suppressions.

  • Bug Fixes

    • Keep canceled items unassigned even when ownership flags are provided.
    • Reject conflicting set/unset for runtime schema fields with runtime‑aware option‑policy checks.
    • Conflict errors now cite the provided runtime field CLI flag for clarity.
  • Refactors

    • Extracted helpers from runUpdate and its apply‑callback in src/cli/commands/update.ts, preserving routing, close‑status handling, dependency/test/doc/file mutations, runtime fields, and changed‑field behavior.
    • Split src/cli/register-mutation.ts actions into named helpers; switched tests to the public parse path; kept defensive string coercion and ensured repeatable schema role/alias flags route as arrays for schema add-status and add-type.

Greptile Summary

This PR completes CodeFactor complexity remediation by decomposing the large runUpdate function and its mutation callback in update.ts, and splitting the six large action closures in register-mutation.ts into named helpers. All existing routing, field mutation, and validation behavior is preserved, with two deliberate behavior fixes: runtime schema fields are now correctly recognised in --unset target parsing for type-policy enforcement, and cancelled items now forcibly clear their assignee even when --assignee is supplied in the same update.

  • update.ts: runUpdate decomposes into ~20 focused helpers; introduces UpdateMutationContext and UpdateClearCollectionDefinition interfaces; adds runtimeFieldRegistry to enforceUpdateOptionsByType/collectProvidedUpdatePolicyOptions to correctly detect runtime field --unset conflicts; moves cancelled-item assignee removal into mutateUpdateDocument.
  • register-mutation.ts: Extracts runCreateAction, runCloseManyAction, runPlanAction, runHistoryCompactAction, runSchemaAction, runCommentsAction plus shared utilities; removes /* c8 ignore next */ suppression on the defensive string-coercion path for alias/role, now covered by new tests.
  • Tests: New integration tests cover repeatable --role/--alias array routing, programmatic string coercion, cancellation-overrides-assignee behaviour, and --unset + runtime field conflict; ESLint complexity suppressions for the refactored functions are removed.

Confidence Score: 5/5

Safe to merge; behaviour-preserving decomposition with targeted fixes, comprehensive test coverage, and all gates passing.

The refactoring faithfully preserves routing, mutation ordering, and validation logic. The two intentional behaviour changes are each backed by new integration tests and documented in the PR description.

The cancelled-item assignee-removal block in src/cli/commands/update.ts (mutateUpdateDocument) deserves a second look to confirm the intended scope of the guard.

Important Files Changed

FilenameOverview
src/cli/commands/update.tsLarge refactor extracting helpers from runUpdate and its mutate callback. Adds runtimeFieldRegistry to enforceUpdateOptionsByType/collectProvidedUpdatePolicyOptions (correct fix), extracts context types, and splits mutation logic into focused helpers.
src/cli/register-mutation.tsExtracts large inline action closures into named top-level helpers. Introduces shared utilities. Logic is preserved faithfully.
tests/unit/commands/update-command.spec.tsAdds tests for collectProvidedUpdatePolicyOptions with runtime field registry, cancellation-overrides-assignee behaviour, and --unset + runtime field conflict.
tests/unit/commands/register-commands.spec.tsAdds tests for repeatable --role/--alias array routing and defensive string coercion.
eslint-suppressions.jsonRemoves complexity suppressions for the refactored functions.
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
src/cli/commands/update.ts:2057-2065
The cancelled-item assignee removal check no longer requires `options.status !== undefined`. In the original code the guard was inside `if (options.status !== undefined)`, so it only fired when the update itself was transitioning the item to cancelled. The new placement runs unconditionally after all mutations, meaning any update to an item that is **already in cancelled state with an existing assignee** (e.g., data imported from another tool, or a re-opened-then-re-cancelled item) will silently clear the `assignee` field. Adding the original guard makes the scope explicit and prevents unintended side-effects on non-status updates.

```suggestion
  if (
    context.options.status !== undefined &&
    normalizeStatusInput(document.metadata.status, context.statusRegistry) === context.statusRegistry.canceled_status &&
    document.metadata.assignee !== undefined
  ) {
    delete document.metadata.assignee;
    if (!changedFields.includes("assignee")) {
      changedFields.push("assignee");
    }
  }
```

### Issue 2 of 2
src/cli/register-mutation.ts:658-664
The `pm-6mit` comment explaining why `step` is intentionally absent from the alias pairs was preserved in the original inline closure but is lost in the extracted function. Without it, a future reader may assume the omission is a mistake and re-add the alias, re-introducing the regression described in `pm-6mit`.

```suggestion
function normalizePlanAliases(options: Record<string, unknown>): Record<string, unknown> {
  const planOptions: Record<string, unknown> = { ...options };
  const aliasPairs: Array<[string, string]> = [
    ["blocked_by", "blockedBy"],
    ["resume_context", "resumeContext"],
    ["from_search", "fromSearch"],
    // pm-6mit: "step" is intentionally not aliased to stepTitle here — it is a
    // first-class repeatable option normalized inside runPlan.
    ["step_title", "stepTitle"],
```

Reviews (6): Last reviewed commit: "Use runtime field CLI flag in conflict e..." | Re-trigger Greptile