simple mode: KiroCrew default, curated pack list with UI hints - #81
Conversation
- KiroCrew is now the default selection in simple mode - Simple mode shows only: KiroCrew, OpenClaw, Hermes, Claude Code, Codex, Kiro-CLI, Troika - KiroCrew labeled [WebUI], all others labeled [Terminal] (as prefix for scannability) - Advanced mode unchanged (still shows all packs, defaults to OpenClaw) Review fixes (Sol + Sonnet): - M1: Iterate simple_packs in declared order, not registry order - M2: Fallback to first item if default pack missing from registry - M3: AUTO_YES non-interactive auto-selects kirocrew without gum - L1: webui_packs array for extensibility (not hardcoded if/else) - L2: Hoisted local declarations out of loop - L3: UI type as prefix [WebUI]/[Terminal] for visual scanning - Fix PACK_NAME extraction to strip prefix labels
0ddd284 to
a6625f1
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a6625f114a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if [[ "$INSTALL_MODE" == "simple" && "$AUTO_YES" == true ]]; then | ||
| PACK_NAME="kirocrew" |
There was a problem hiding this comment.
When the installer is invoked with bare -y and no --pack or --kiro-from-secret, this branch now selects KiroCrew, while the later credential collection is skipped whenever AUTO_YES=true (install.sh:3179-3182). The KiroCrew installer explicitly reports that Kiro authentication is required without an API key (packs/kirocrew/install.sh:709-713), so the documented unattended path completes with a dashboard whose agent cannot execute work until someone manually connects and performs device-flow login. Keep an IAM-authenticated pack as the credential-free -y default, or require/provision Kiro credentials before automatically selecting KiroCrew.
Useful? React with 👍 / 👎.
| PACK_NAME="openclaw" | ||
| warn "kirocrew not found in registry — falling back to openclaw" | ||
| fi | ||
| ok "Agent: ${PACK_NAME} (auto-selected)" |
There was a problem hiding this comment.
For bare non-interactive installs, this early return bypasses both existing experimental-warning paths even though kirocrew is marked experimental: true in the registry. Interactive users see (experimental) and explicit --pack kirocrew users receive the rough-edges warning, but unattended users—the only users who do not choose the pack themselves—are silently moved from the stable OpenClaw default to an experimental pack. Consult PACK_EXPERIMENTAL before returning so this default change is visible in automation logs.
Useful? React with 👍 / 👎.
Pack-agnostic design for configure_webui_auth(): - Cognito Managed Login + PKCE (no client secret) - AdminCreateUserOnly (no self-signup) - Server-side JWT enforcement requirements - Reusable by any future WebUI pack - Security considerations, edge cases, cleanup plan Incorporates Sol review feedback on: - C1: Enforcement defined (JWT validation on all endpoints) - C2: OAuth flow clarified (auth-code + PKCE, not SRP) - C3: App client created for both new and existing pools - C4: No-auth gated on network isolation - C5: HTTPS required for remote access
- Add experimental warning in AUTO_YES auto-select path - Validate openclaw fallback exists (fail clearly if neither default found) - Declare wp local (prevent scope leak) - Guard against empty gum_items with clear error message
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: abacc957ff
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| aws cognito-idp create-user-pool-client \ | ||
| --user-pool-id "${POOL_ID}" \ | ||
| --client-name "${pack_name}-webui" \ | ||
| --generate-secret # ← NO: public client, no secret |
There was a problem hiding this comment.
When this documented snippet is executed, the inline comment terminates the continued shell command, so the AWS invocation ends after --generate-secret; it creates a secret-bearing client, and the following --no-generate-secret line runs as a separate command and fails. This contradicts the public-client PKCE design and prevents all subsequent OAuth settings from being applied, so move the explanation outside the command and retain only --no-generate-secret.
Useful? React with 👍 / 👎.
KiroCrew is now the default in simple mode — no longer experimental.
- Update registry.yaml (source of truth) for kirocrew experimental=false - Regenerate registry.json via sync script - Fix invalid CLI options in design doc (remove --allowed-o-auth-flows-with-pkce, remove contradictory --generate-secret) - Fix fail-open: pack must fail clearly if it lacks auth enforcement - All tests pass locally (sync: 46/46, contracts: 177/177)
502c215 to
a473722
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a473722321
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| --allowed-o-auth-flows code \ | ||
| --allowed-o-auth-scopes openid email \ |
There was a problem hiding this comment.
When this provisioning command is used, the client does not opt in to Cognito's OAuth authorization-server features, so the documented Managed Login authorization-code flow cannot work despite specifying code, scopes, and callback URLs. The AWS CLI reference states that --allowed-o-auth-flows-user-pool-client must be set to use OAuth 2.0 features; add that flag as the repository's existing Cognito example does.
Useful? React with 👍 / 👎.
| import secrets, string | ||
| alphabet = string.ascii_letters + string.digits + '!@#$%&*' | ||
| print(''.join(secrets.choice(alphabet) for _ in range(16))) |
There was a problem hiding this comment.
When the generated password happens to omit a category, admin-set-user-password rejects it because the pool policy above requires uppercase, lowercase, numeric, and symbol characters. Drawing all 16 characters independently from the combined alphabet has about a 22% chance of missing at least one required category, making initial-user provisioning intermittently fail; generate one character from each class and shuffle the result instead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a45e315860
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| --user-pool-tags "loki:managed=true,loki:pack=${pack_name},loki:env=${ENV_NAME}" \ | ||
| --region "$DEPLOY_REGION" --output json 2>/dev/null) \ | ||
| || fail "Cognito user pool creation failed; verify cognito-idp permissions." | ||
| pool_id=$(echo "$pool_json" | json_field Id) |
There was a problem hiding this comment.
When auth setup creates a new pool, create-user-pool returns the ID under .UserPool.Id, but json_field Id reads the nonexistent top-level .Id, so pool_id becomes null and app-client creation fails. The existing-pool path also cannot complete because line 2202 similarly reads .ClientId instead of .UserPoolClient.ClientId; parse both nested response fields so every auth-enabled installation does not terminate after creating partial Cognito resources.
Useful? React with 👍 / 👎.
| export WEBUI_AUTH_ENABLED="true" WEBUI_COGNITO_POOL_ID="$pool_id" WEBUI_COGNITO_CLIENT_ID="$client_id" | ||
| export WEBUI_COGNITO_DOMAIN="${domain_prefix}.auth.${DEPLOY_REGION}.amazoncognito.com" | ||
| export WEBUI_COGNITO_REGION="$DEPLOY_REGION" WEBUI_CALLBACK_URL="$callback_url" WEBUI_LOGOUT_URL="$logout_url" |
There was a problem hiding this comment.
When this function succeeds, these exports exist only in the local installer process: they are absent from PARAM_CFN_NAMES/PARAM_VALUES, the CloudFormation userdata, and the KiroCrew pack, and a repository-wide search finds no consumer outside this function. Consequently the deployed gateway cannot know the pool, client, or domain, so the newly printed Cognito credentials never authenticate to the WebUI even though the installer reports it as protected.
Useful? React with 👍 / 👎.
| --supported-identity-providers COGNITO --allowed-o-auth-flows code \ | ||
| --allowed-o-auth-scopes openid email --callback-urls "[\"${callback_url}\"]" \ |
There was a problem hiding this comment.
When this executable provisioning path is used for Managed Login, it configures authorization-code flows and scopes but omits --allowed-o-auth-flows-user-pool-client, which the AWS CLI documentation requires to activate OAuth 2.0 features for the client. Thus the authorization endpoint cannot run the intended code flow; the fresh evidence beyond the earlier documentation finding is that the same omission is now present in the actual installer command.
Useful? React with 👍 / 👎.
| local callback_url="http://localhost:${webui_port}${callback_path}" | ||
| local logout_url="http://localhost:${webui_port}/" |
There was a problem hiding this comment.
For the normal KiroCrew CloudFormation deployment, deploy/cloudformation/template.yaml:632-667 exposes the dashboard through an HTTPS CloudFront distribution, but this app client registers only a callback on http://localhost:5476. A user beginning login from the deployed dashboard is therefore redirected to port 5476 on their own machine instead of back to the deployment, so the Cognito flow cannot complete unless they independently establish and use a local tunnel.
Useful? React with 👍 / 👎.
| fail "Invalid --webui-email value: ${user_email}" | ||
| fi | ||
| local password | ||
| password=$(python3 -c 'import secrets,string; print("".join(secrets.choice(string.ascii_letters+string.digits+"!@#$%&*") for _ in range(16)))') |
There was a problem hiding this comment.
When random selection omits any character class, admin-set-user-password rejects this password because the newly created pool requires uppercase, lowercase, numbers, and symbols. With this alphabet and 16 independent draws, that occurs about 25% of the time, after the pool, client, domain, and user have already been created; the fresh evidence beyond the earlier design-document finding is that the probabilistic generator is now executable installer code. Generate at least one character from each required class and shuffle the result.
Useful? React with 👍 / 👎.
…lay, tags format)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bf0db8ca42
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| allow_admin=$(echo "$pool_cfg" | jq -r '.UserPool.AdminCreateUserConfig.AllowAdminCreateUserOnly // true') | ||
| [[ "$allow_admin" != true ]] && fail "Existing pool ${pool_id} permits self-signup; choose a pool with admin-only user creation." |
There was a problem hiding this comment.
When an existing pool explicitly has AllowAdminCreateUserOnly: false, jq's // operator treats that boolean as absent and substitutes true, so the subsequent guard accepts the pool. Any user can then self-register and obtain tokens for this public app client, violating the installer’s admin-only authentication guarantee; default only null or a missing path rather than false.
Useful? React with 👍 / 👎.
| local ts_suffix; ts_suffix=$(date +%s | tail -c 4) | ||
| ENV_NAME="${PACK_NAME}-$((existing_count + 1))-${ts_suffix}" | ||
| LOKI_WATERMARK="$ENV_NAME" | ||
| [[ "$PACK_NAME" == "kirocrew" ]] && configure_webui_auth "$PACK_NAME" 5476 "/auth/callback" |
There was a problem hiding this comment.
For an interactive KiroCrew install, this call creates the pool, app client, domain, and user during configuration, before show_summary asks whether to deploy. Selecting “Change settings” reruns configuration and can provision another set, while aborting or any later quota/deployment failure leaves the first set orphaned; collect the choices here but perform these side effects only after final confirmation, as the installer already does for pack credentials.
Useful? React with 👍 / 👎.
| --admin-create-user-config '{"AllowAdminCreateUserOnly":true}' \ | ||
| --auto-verified-attributes email --username-attributes email \ | ||
| --schema '[{"Name":"email","Required":true,"Mutable":true}]' \ | ||
| --user-pool-tags "{\"loki:managed\":\"true\",\"loki:pack\":\"${pack_name}\",\"loki:env\":\"${ENV_NAME}\"}" \ |
There was a problem hiding this comment.
Every newly created pool is tagged as belonging to the deployment, but the repository-wide Cognito command search finds no cleanup in uninstall.sh, whose discovery and teardown are driven by the managed VPC. Consequently a normal successful uninstall deletes the stack/VPC but permanently leaves this pool, its domain, app client, and login user behind; extend teardown to discover these tags by environment and delete the Cognito resources.
Useful? React with 👍 / 👎.
- #2: Add --allowed-o-auth-flows-user-pool-client to enable OAuth/managed login - #3: Password generation guarantees uppercase, lowercase, digit, and symbol - #4: Defer Cognito resource creation until after user confirms deployment (prevents orphaned resources on cancel/change-settings) - #5: Check for existing domain on pool before creating new one (reuse) - #1: Write WEBUI config to SSM Parameter Store so instance can read during bootstrap (fixes auth enforcement gap)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dd7b27a811
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if [[ "${AUTO_YES:-false}" == true && -z "$user_email" ]]; then | ||
| fail "WebUI auth in non-interactive mode requires --webui-email <email>" |
There was a problem hiding this comment.
When the installer is invoked with bare -y, choose_pack now selects KiroCrew and enables deferred WebUI auth, but this guard then unconditionally calls fail because no --webui-email was supplied, so the advertised “accept defaults; skip prompts” path never reaches deployment. Fresh evidence beyond the earlier unattended-default finding is that the newly added deferred Cognito setup now executes this mandatory-email guard before deployment; either retain a pack that needs no extra input as the unattended default or make Cognito provisioning optional without the flag.
Useful? React with 👍 / 👎.
| # Deferred WebUI auth: create Cognito resources only after user confirms deployment | ||
| if [[ "${WEBUI_AUTH_DEFERRED:-false}" == true ]]; then | ||
| configure_webui_auth "$PACK_NAME" 5476 "/auth/callback" |
There was a problem hiding this comment.
When advanced mode uses the CloudFormation Console deployment method, this call creates the pool, client, domain, and user before main invokes deploy_console, which merely opens the stack-review URL and then exits; closing that browser page therefore leaves all Cognito resources behind without any deployment. Fresh evidence after the earlier provisioning-timing finding is that the deferred block still runs before the console handoff rather than after actual stack creation; provision these resources through the stack or only after deployment can be confirmed.
Useful? React with 👍 / 👎.
P1 fixes: - Fix jq // operator treating explicit false as absent (self-registration check) - Add post-deploy Cognito callback URL update with CloudFront domain - Unattended mode: fall back to openclaw (v1 = interactive wizard only) - Non-interactive without --webui-email: skip auth gracefully (TODO for future) - Console deploy: show manual callback URL update instructions P2 fixes: - Docs: add --allowed-o-auth-flows-user-pool-client to app client example - Docs: fix password generation to guarantee all character classes Deferred (intentional): - Cognito cleanup on uninstall (future iteration) - Full unattended KiroCrew support (requires --webui-email plumbing)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8ff6d7ada2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| cf_url=$(aws cloudformation describe-stacks --stack-name "${ENV_NAME}" \ | ||
| --region "$DEPLOY_REGION" --output json 2>/dev/null \ | ||
| | jq -r '.Stacks[0].Outputs[] | select(.OutputKey=="CloudFrontURL" or .OutputKey=="DashboardURL" or .OutputKey=="WebUIURL") | .OutputValue' \ |
There was a problem hiding this comment.
For every CLI KiroCrew deployment with WebUI auth enabled, deploy_cfn_stack creates ${ENV_NAME}-stack, but this query asks for ${ENV_NAME}; because the script uses set -euo pipefail, the resulting describe-stacks error aborts the installer after the stack has been created and before bootstrap completion is reported. The fresh evidence beyond the earlier callback comment is this exact post-deployment lookup mismatch; additionally, the inspected template names the relevant output KiroCrewDashboardUrl, not any of the three keys selected here. Use STACK_NAME and the template's exact output key so callback registration can run.
Useful? React with 👍 / 👎.
1 participant
Changes
[WebUI], all others labeled[Terminal]Why
KiroCrew includes a web dashboard out of the box — the best default for users who want a visual interface. Terminal-only packs are still available but clearly labeled.