Skip to content

Add guided agent configurator#156

Open
TsukinowaRin wants to merge 1 commit into
yohey-w:mainfrom
TsukinowaRin:codex/upstream-config-cui
Open

Add guided agent configurator#156
TsukinowaRin wants to merge 1 commit into
yohey-w:mainfrom
TsukinowaRin:codex/upstream-config-cui

Conversation

@TsukinowaRin
Copy link
Copy Markdown

Summary

Adds a small guided configurator for assigning Shogun agent CLI types without hand-editing config/settings.yaml.

  • Adds scripts/configure_agents.py for CUI and non-interactive configuration.
  • Adds Linux/macOS/Windows launchers: Configure-Agents.sh, Configure-Agents.command, and Configure-Agents.bat.
  • Documents the configurator in English and Japanese README script references and formation setup.
  • Adds unit tests for the configurator and launchers.

Behavior

The configurator asks in this order:

  1. cli.default
  2. Shogun
  3. Karo
  4. Gunshi
  5. active Ashigaru count
  6. each Ashigaru CLI type

It preserves existing model, variant, and thinking settings by default so current model routing remains intact. Users can pass --reset-model-prefs when they intentionally want to clear those detailed fields.

Validation

  • bash -n Configure-Agents.sh Configure-Agents.command scripts/switch_cli.sh shutsujin_departure.sh
  • python3 -m py_compile scripts/configure_agents.py
  • bats tests/unit/test_configure_agents.bats tests/unit/test_configure_agent_launchers.bats
  • bats tests/unit/test_cli_adapter.bats tests/unit/test_switch_cli.bats tests/unit/test_agent_registry.bats
  • cmd.exe /c Configure-Agents.bat --dry-run --default claude --ashigaru-count 1 --shogun claude --karo claude --gunshi claude --ashigaru1 claude
  • git diff --check

@TsukinowaRin TsukinowaRin marked this pull request as ready for review May 26, 2026 03:28
Copy link
Copy Markdown
Owner

@yohey-w yohey-w left a comment

Choose a reason for hiding this comment

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

Clean and useful, @TsukinowaRin. The three-platform launcher approach (sh/bat/command) covers Linux, Windows WSL2, and macOS Finder — that's the right scope for a setup tool.

What's good:

  • configure_agents.py preserves model, variant, thinking, reasoning_effort, and recommended_model fields — users won't lose carefully tuned model routing when changing CLI type
  • Non-interactive mode via --default, --shogun, --ashigaru-count etc. makes it scriptable for CI/bootstrap
  • Configure-Agents.bat correctly resolves WSL path via wslpath and delegates to the Python script — no duplication

One note:

configure_agents.py has ALLOWED_CLIS = ('claude', 'codex', 'copilot', 'kimi', 'opencode') — it's missing cursor (PR #155) and antigravity (PR #154). Once those CLIs land, this list will need a patch. Low priority, but worth a follow-up.

Merging is safe.

Copy link
Copy Markdown
Owner

@yohey-w yohey-w left a comment

Choose a reason for hiding this comment

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

Revisiting my earlier approval — the ALLOWED_CLIS list is not MECE with the system's actual CLI support, and for a tool whose entire purpose is to enumerate valid options, that's a core defect, not a low-priority footnote.

Required change before merge:

scripts/configure_agents.py has:

ALLOWED_CLIS = ('claude', 'codex', 'copilot', 'kimi', 'opencode')

After PRs #154 (Antigravity) and #155 (Cursor) land, the system supports 7 CLIs. The configurator would be wrong on day 1 after those merges — silently offering only 5 of 7 valid options.

Options:

  1. Add cursor and antigravity to ALLOWED_CLIS now (coordinate with #154/#155 authors)
  2. Or derive the list from cli_adapter.sh's CLI_ADAPTER_ALLOWED_CLIS at runtime to avoid the sync problem entirely

The deeper issue is that 4 places currently hold copies of this list (cli_adapter.sh, inbox_watcher.sh, configure_agents.py, switch_cli.sh). Every new CLI requires 4 separate edits. A single source of truth would prevent this class of bug — but even without that refactor, the list here must match what the adapter accepts before merge.

Sorry for the earlier approval — this should have been a required change from the start.

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.

2 participants