Follow-up from the #264 review (finding #2). The web_search and channel cases were fixed in #264 (each step exclusively owns its keys, so a clear is safe). The remaining cases are harder because the keys are shared across steps.
Problem
advanceStep calls each step's Apply on every forward pass and there is no undo on back-navigation. Set-style Applies self-heal on re-advance; conditional map writes don't. With back-nav now enabling redo, these leak:
- ProviderStep.Apply writes
OPENAI_API_KEY / ANTHROPIC_API_KEY / GEMINI_API_KEY for the primary provider. Change provider on redo (OpenAI → Anthropic) and the stale OPENAI_API_KEY persists into .env.
- FallbackStep.Apply writes the same
*_API_KEY names for fallbacks. Remove a fallback on redo and its key persists.
- SkillsStep.Apply writes skill-required env vars (
ctx.EnvVars[k]). Deselect a skill on redo and its env vars persist.
Why it's not a trivial clear
ProviderStep and FallbackStep share the *_API_KEY namespace: if the primary is OpenAI and a fallback is also OpenAI, both write OPENAI_API_KEY. So a step can't just delete "its" provider keys at the top of Apply — it might wipe a key another step legitimately set (and there's a latent last-writer-wins collision here independent of back-nav).
Options
- Per-step key ownership tracking — each step records exactly the keys it wrote last pass and clears only those on re-Apply. Handles the overlap but adds bookkeeping.
- Rebuild the context from scratch on each advance — reset
EnvVars/ChannelTokens/BuiltinTools and re-run every completed step's Apply in order. Cleanest mental model; makes Apply idempotent by construction. Biggest change.
- Namespace the fallback keys (e.g.
FALLBACK_1_OPENAI_API_KEY) so provider/fallback stop colliding, then per-step clears become safe.
Option 2 is the most robust. Recommend it unless the collision in option 3 is worth fixing on its own.
Acceptance
Related: #264 (web_search + channel cases fixed there).
Follow-up from the #264 review (finding #2). The web_search and channel cases were fixed in #264 (each step exclusively owns its keys, so a clear is safe). The remaining cases are harder because the keys are shared across steps.
Problem
advanceStepcalls each step'sApplyon every forward pass and there is no undo on back-navigation. Set-style Applies self-heal on re-advance; conditional map writes don't. With back-nav now enabling redo, these leak:OPENAI_API_KEY/ANTHROPIC_API_KEY/GEMINI_API_KEYfor the primary provider. Change provider on redo (OpenAI → Anthropic) and the staleOPENAI_API_KEYpersists into.env.*_API_KEYnames for fallbacks. Remove a fallback on redo and its key persists.ctx.EnvVars[k]). Deselect a skill on redo and its env vars persist.Why it's not a trivial clear
ProviderStep and FallbackStep share the
*_API_KEYnamespace: if the primary is OpenAI and a fallback is also OpenAI, both writeOPENAI_API_KEY. So a step can't justdelete"its" provider keys at the top of Apply — it might wipe a key another step legitimately set (and there's a latent last-writer-wins collision here independent of back-nav).Options
EnvVars/ChannelTokens/BuiltinToolsand re-run every completed step's Apply in order. Cleanest mental model; makes Apply idempotent by construction. Biggest change.FALLBACK_1_OPENAI_API_KEY) so provider/fallback stop colliding, then per-step clears become safe.Option 2 is the most robust. Recommend it unless the collision in option 3 is worth fixing on its own.
Acceptance
.env.*_API_KEYcollision is resolved or documented.Related: #264 (web_search + channel cases fixed there).