feat(init): simplify init flow — scope removal, two-step plugin selection, /bug-analysis in WORKFLOW_ORDER

[dean0x]

Problem Being Solved

Three init-flow rough edges: an unwanted user/local scope prompt, a missing /bug-analysis entry in the post-install command list, and a single plugin list that conflated workflow and language plugins.

Key Changes to Highlight

• Interactive init always installs on user scope (scope prompt removed; --scope flag kept).
• /bug-analysis now appears in the "Available commands" note, guarded by a coverage test.
• Plugin selection split into two steps: workflow plugins, then language plugins.
• New pure helper partitionSelectablePlugins + exported WORKFLOW_ORDER, both unit-tested.

Breaking Changes

None for scripted use (--plugin, --scope unchanged). Interactive scope choice is removed by design; deep local-scope removal is deferred.

Reviewer Focus Areas

• The bounded re-prompt loop and combined non-empty validation (language-only path).
• isCancel handling on both new multiselects.
• Correct exclusions in partitionSelectablePlugins.

[dean0x]

Redundant as string[] cast (TypeScript review, 88% confidence)

After the isCancel(step1) type guard above, TypeScript has already narrowed step1 from symbol | string[] to string[]. The as string[] assertion is therefore redundant and suppresses future compiler help if the option value type changes.

Fix: Remove the cast—let type narrowing do the work:

workflowSelected = step1;  // was: step1 as string[]

Note: While this cast matches pre-existing convention in the file, the best practice is to remove it (and ideally update similar pre-existing casts in a follow-up).

---

This is a review finding from devflow code-review (9 reviewers). See full analysis at .devflow/docs/reviews/

[dean0x]

Redundant as string[] cast (TypeScript review, 88% confidence)

Same issue as above: isCancel(step2) narrows the type to string[], making this assertion redundant.

Fix:

languageSelected = step2;  // was: step2 as string[]

---

This is a review finding from devflow code-review (9 reviewers). See full analysis at .devflow/docs/reviews/

[dean0x]

Interactive selection loop logic is untested (Testing review, 88% confidence)

The bounded retry (MAX_ATTEMPTS = 3), empty-bucket skip, isCancel handling, and combine/accept logic are implemented inline here and only statically verified. The testable decision logic should be extracted for unit testing.

Impact: Three behaviors carry regression risk with zero test coverage:

1. Retry-then-cancel boundary (does attempt 3 with empty selection actually exit?)
2. Empty-bucket skip (is workflowSelected correctly left [] if no workflow choices?)
3. Combine/accept rule

Fix: Extract a pure reducer like:

function combineSelection(
  workflowSelected: string[], 
  languageSelected: string[]
): { plugins: string[]; accepted: boolean } {
  const combined = [...workflowSelected, ...languageSelected];
  return { plugins: combined, accepted: combined.length > 0 };
}

Also extract shouldRetry(attempt: number, maxAttempts: number, accepted: boolean): boolean and unit-test both. This mirrors the existing parsePluginSelection extraction pattern and keeps only the p.multiselect/isCancel/exit plumbing inline (reasonably left untested).

---

This is a review finding from devflow code-review (9 reviewers). See full analysis at .devflow/docs/reviews/

[dean0x]

WORKFLOW_ORDER regression guard is directionally asymmetric (Testing review, 82% confidence)

This test asserts that every workflow plugin command appears in WORKFLOW_ORDER, but does NOT check the reverse: that every entry in WORKFLOW_ORDER is still a known command. This misses the symmetric case—a stale or typo'd WORKFLOW_ORDER entry that no longer maps to any plugin command would go undetected.

Fix: Add a reverse-direction assertion:

const allKnownCommands = new Set(
  DEVFLOW_PLUGINS.flatMap(p => p.commands)
);
WORKFLOW_ORDER.forEach(cmd => {
  expect(allKnownCommands.has(cmd)).toBe(true);
});

Together with the existing forward check, this makes the guard bidirectional and catches both dropped commands and stale/renamed entries.

---

This is a review finding from devflow code-review (9 reviewers). See full analysis at .devflow/docs/reviews/

[dean0x]

Review Summary — All 9 Reviewers APPROVED_WITH_CONDITIONS

Blockers: None — merge ready once should-fix items above are addressed.

Merge Recommendation: APPROVED_WITH_CONDITIONS (82% confidence overall)

---

Key Strengths

• Strong extraction of partitionSelectablePlugins with comprehensive test coverage
• Correct placement of WORKFLOW_ORDER in the registry module (plugins.ts)
• Improved layering: init.ts now consumes abstractions rather than duplicating classification rules
• All 39 tests passing; clean TypeScript strict mode

Should-Fix Items (4 inline comments above)

1. TypeScript (88%): Remove redundant as string[] casts at lines 342, 357
2. Testing (88%): Extract and unit-test interactive selection-loop decision logic (lines 322-373)
3. Testing (82%): Make WORKFLOW_ORDER regression guard bidirectional (line 400)

---

Lower-Confidence Observations (summary format)

Architecture clarity (82%, MEDIUM)

• Selection loop body mixes "collect selection" with "decide to terminate" (init.ts ~367-372). Optional refactor for clarity: separate the terminal decision (isLastAttempt check) from retry warning.

Implicit language-bucket contract (80%, LOW)

• Bucket classified as "language" by commands.length === 0 rather than explicit property. No current defect; future non-language command-less plugins would silently land in "Step 2" UI section. Consider either renaming bucket to commandless or asserting optional === true for everything in the language bucket.

Test oracle coupling (65-70%, LOW)

• Immutability test is shallow (array copy but nested arrays not verified).
• Order-preservation test recomputes expected order with same predicate as implementation (couples test to impl).

---

All Reviewers

• architecture.md: APPROVED (9/10) — Layering improved, WORKFLOW_ORDER correctly placed
• complexity.md: APPROVED (9/10) — Clear structure, no unnecessary nesting
• consistency.md: APPROVED (9/10) — Naming and patterns consistent with codebase
• performance.md: APPROVED (10/10) — No regressions, O(1) exclusion check via Set
• regression.md: APPROVED_WITH_CONDITIONS (8/10) — Asymmetric guard needs reversal
• reliability.md: APPROVED (9/10) — Bounded loop (MAX_ATTEMPTS=3), no unbounded operations
• security.md: APPROVED (10/10) — No sensitive data exposure, proper input narrowing
• testing.md: APPROVED_WITH_CONDITIONS (8/10) — Selection loop logic extraction needed
• typescript.md: APPROVED (9/10) — Redundant casts need removal

---

Generated by devflow code-review — [parallel 9-reviewer architecture, complexity, consistency, performance, regression, reliability, security, testing, typescript]

[dean0x]

Review Cycle 2: All Clear ✓

Reviewers: 9/9 approved (security, architecture, performance, complexity, consistency, regression, testing, reliability, typescript)
Status: Zero blocking issues, zero should-fix issues at ≥80% confidence

All nine reviewers have completed their analysis with unanimous APPROVED recommendations. No inline comments needed — all code issues have met the acceptance criteria for this PR.

Reviewer Scores

• Architecture: 9/10 — Clean separation of concerns, helpers properly extracted
• Security: 10/10 — No attack surface introduced
• Complexity: 9/10 — Well-factored, within all thresholds
• Consistency: 9/10 — Matches existing codebase patterns
• Performance: 10/10 — Algorithmically clean, no O(n²) issues
• Reliability: 10/10 — Loop bounds verified, graceful termination on exhaustion
• Regression: 10/10 — All WORKFLOW_ORDER guards in place, no dropped functionality
• Testing: 9/10 — Pure helpers well-covered with behavior-focused tests
• TypeScript: 9/10 — Type-sound with strict mode

Low-Confidence Suggestions (For Author's Optional Consideration)

The following items are marked <65% confidence — noted for quality but not blocking:

From Architecture (65-62%):

• Duplicated EXCLUDED set could eventually unify to a single source of truth via a category field on PluginDefinition (avoids PF-005)
• partitionSelectablePlugins couples "language" semantics to "command-less" structure — already documented inline as an intentional simplification

From Complexity (65%):

• shouldRetry's accepted parameter is always false at its sole callsite but exists deliberately for unit-test coverage (intentional testability tradeoff)

From Consistency (65%):

• JSDoc "pure function" tagline phrasing varies slightly across the three new helpers — harmonizing would be a style improvement but is below the tracking bar

From Regression (70%):

• Interactive local-scope install path is now unreachable by design (applies ADR-010: users must use --scope local to reach that path)

From Testing (65-62%):

• combineSelection lacks an immutability assertion on inputs (low value given the obvious non-mutating spread)
• Empty-bucket loop-skip interaction end-to-end untested, but covered indirectly via helper unit tests and simple branch wiring

From TypeScript (65%):

• WORKFLOW_ORDER could be typed readonly string[] or as const for mutation hardening (currently correct but not hardened)

All of the above are stylistic or design-hardening suggestions. Core functionality is solid and all architectural/reliability/security concerns have been addressed.

---

Claude Code | Review Cycle 2 | 2026-06-01