Skip to content

fix(cli): show Pi community provider in archon setup multiselect (#1526)#1

Closed
YrFnS wants to merge 1 commit intodevfrom
fix/issue-1526-pi-community-provider-setup
Closed

fix(cli): show Pi community provider in archon setup multiselect (#1526)#1
YrFnS wants to merge 1 commit intodevfrom
fix/issue-1526-pi-community-provider-setup

Conversation

@YrFnS
Copy link
Copy Markdown
Owner

@YrFnS YrFnS commented May 2, 2026

Summary

Describe this PR in 2-5 bullets:

  • Problem: The archon setup wizard hardcodes only Claude and Codex in the AI assistant multiselect. The Pi community provider (builtIn: false) is never shown, so users cannot select it as their default assistant during setup.
  • Why it matters: Users cannot select Pi as their default assistant during setup, limiting their provider choices.
  • What changed: collectAIConfig() now builds multiselect options dynamically from getRegisteredProviders(), supports community providers, and skips auth collection for non-built-in providers.
  • What did not change: SetupConfig[\"ai\"] interface, env generation logic, existing tests.

UX Journey

Before

○ Setup wizard shows only hardcoded providers
  ◻ Claude (Recommended)
  ◻ Codex
  Pi (community) never appears

After

○ Setup wizard shows dynamically registered providers
  ◻ Claude (Recommended) (Anthropic Claude Code SDK)
  ◻ Codex (OpenAI Codex SDK)
  ◻ Pi (community)           ← NEW
  [Select one or more]

Architecture Diagram

Connection inventory

From To Status Notes
collectAIConfig() getRegisteredProviders() modified Now dynamically reads providers
collectAIConfig() collectClaudeAuth() modified Only called for built-in providers
collectAIConfig() collectCodexAuth() modified Only called for built-in providers
collectAIConfig() generateEnvContent() unchanged Uses same claude/codex booleans

Label Snapshot

  • Risk: risk: low
  • Size: size: S
  • Scope: cli
  • Module: cli:setup

Change Metadata

  • Change type: feature
  • Primary scope: cli

Linked Issue

Validation Evidence (required)

bun run type-check  # ✅ 0 errors
bun run lint        # ✅ 0 errors
bun run format:check # ✅ clean
bun run test        # ✅ all packages pass
  • Evidence provided: Tests pass, ESLint passes, TypeScript compiles
  • If any command is intentionally skipped: None

Security Impact (required)

  • New permissions/capabilities? No
  • New external network calls? No
  • Secrets/tokens handling changed? No
  • File system access scope changed? No

Compatibility / Migration

  • Backward compatible? Yes
  • Config/env changes? No
  • Database migration needed? No

Human Verification (required)

  • Verified scenarios: Type check, lint, format, tests all pass
  • Edge cases checked: Multiple built-in, single built-in, community-only, mixed selections
  • What was not verified: Manual interactive testing in a real terminal session

Side Effects / Blast Radius (required)

  • Affected subsystems/workflows: CLI setup wizard only
  • Potential unintended effects: None - community providers skip auth silently as designed
  • Guardrails/monitoring for early detection: N/A

Rollback Plan (required)

  • Fast rollback command/path: git revert HEAD
  • Feature flags or config toggles: None needed
  • Observable failure symptoms: Setup wizard would show old hardcoded list

Risks and Mitigations

None - this is a straightforward enhancement that uses existing registry infrastructure.

…eam00#1526)

- Replace hardcoded Claude/Codex options with dynamic provider list
- Built-in providers shown first with SDK hints
- Community providers shown with 'community' hint
- Skip auth collection for non-built-in providers (Pi manages its own auth via ~/.pi/)
- Set defaultAssistant correctly for community-only or mixed selections
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the AI assistant configuration in the setup command to dynamically support both built-in and community providers. Key changes include dynamic multiselect option generation and improved validation of tool availability. The review feedback highlights several areas for improvement: ensuring the default assistant logic only considers successfully configured providers, optimizing performance by reusing the registered providers list, and refining the selection priority to favor active built-in assistants over community ones.

Comment on lines +873 to +875
const builtinSelected = selectedProviders.filter(id =>
getRegisteredProviders().find(p => p.id === id && p.builtIn)
);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The builtinSelected list currently includes providers that the user might have opted to skip (e.g., if the binary was missing and they chose to continue without it). This can lead to a skipped provider being set as the default assistant or appearing in the selection prompt. It should be filtered based on finalHasClaude and finalHasCodex to reflect only available providers.

Suggested change
const builtinSelected = selectedProviders.filter(id =>
getRegisteredProviders().find(p => p.id === id && p.builtIn)
);
const activeBuiltins = [];
if (finalHasClaude) activeBuiltins.push('claude');
if (finalHasCodex) activeBuiltins.push('codex');

Comment on lines +878 to +883
if (builtinSelected.length > 1) {
// Multiple built-in providers: prompt user to choose default
const providerChoices: { value: string; label: string }[] = [];
for (const id of builtinSelected) {
const p = getRegisteredProviders().find(p => p.id === id);
if (p) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Use the filtered activeBuiltins list and the existing providers variable to ensure only available providers are considered for the default assistant selection.

  let defaultAssistant: string;
  if (activeBuiltins.length > 1) {
    // Multiple built-in providers: prompt user to choose default
    const providerChoices: { value: string; label: string }[] = [];
    for (const id of activeBuiltins) {
      const p = providers.find(p => p.id === id);
      if (p) {

Comment on lines +902 to 911
} else if (builtinSelected.length === 1) {
// Single built-in provider selected
defaultAssistant = builtinSelected[0];
} else if (selectedProviders.length > 0) {
// Only community providers selected — use first one
defaultAssistant = selectedProviders[0];
} else {
// Fallback to first available built-in
defaultAssistant = getRegisteredProviders().find(p => p.builtIn)?.id ?? 'claude';
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The logic for determining the defaultAssistant should prioritize active built-in providers and then fall back to selected community providers. The current implementation might pick a skipped built-in provider if it was the first one in selectedProviders. Using activeBuiltins and communitySelected ensures a valid configuration.

Suggested change
} else if (builtinSelected.length === 1) {
// Single built-in provider selected
defaultAssistant = builtinSelected[0];
} else if (selectedProviders.length > 0) {
// Only community providers selected — use first one
defaultAssistant = selectedProviders[0];
} else {
// Fallback to first available built-in
defaultAssistant = getRegisteredProviders().find(p => p.builtIn)?.id ?? 'claude';
}
} else if (activeBuiltins.length === 1) {
// Single built-in provider selected
defaultAssistant = activeBuiltins[0];
} else if (communitySelected.length > 0) {
// Only community providers selected — use first one
defaultAssistant = communitySelected[0];
} else {
// Fallback to first available built-in
defaultAssistant = providers.find(p => p.builtIn)?.id ?? 'claude';
}

Comment on lines +836 to +838
const hasCommunityProvider = selectedProviders.some(
id => !getRegisteredProviders().find(p => p.id === id)?.builtIn
);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The getRegisteredProviders() function is called repeatedly within the some loop, which is inefficient. Since the providers variable is already available from line 698, it should be reused. Additionally, pre-calculating the list of selected community providers here will simplify the defaultAssistant logic later.

Suggested change
const hasCommunityProvider = selectedProviders.some(
id => !getRegisteredProviders().find(p => p.id === id)?.builtIn
);
const communitySelected = selectedProviders.filter(id =>
providers.find(p => p.id === id && !p.builtIn)
);
const hasCommunityProvider = communitySelected.length > 0;

@YrFnS YrFnS closed this May 2, 2026
@YrFnS YrFnS deleted the fix/issue-1526-pi-community-provider-setup branch May 2, 2026 08:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(setup): Pi (community) provider missing from archon setup AI assistant selection

1 participant