fix(workspace): make teardown script resilient to missing deps and env vars

[kkjcheng]

Summary

• Source root .env via SUPERSET_ROOT_PATH (like setup does) so NEON_PROJECT_ID is available during teardown
• Degrade gracefully when neonctl or docker are missing — skip steps with warnings instead of hard-failing
• Dependency check is now informational only (each step already guards itself)

Closes #1394

Test plan

• Run teardown without neonctl installed — should warn and skip Neon branch deletion instead of failing
• Run teardown without NEON_PROJECT_ID in local .env but with it in root .env — should load it from root
• Run teardown with all deps present — should behave as before

Summary by CodeRabbit

• Chores
  • Teardown is now non-fatal: missing dependencies or external tools produce warnings and skip affected steps instead of failing.
  • Environment loading supports optional root and local env files with flexible fallback; teardown proceeds with warnings when none are found.
  • Summary output records warnings/skips for non-fatal conditions rather than halting the process.

[coderabbitai]

📝 Walkthrough

Walkthrough

Updated .superset/teardown.sh to source $SUPERSET_ROOT_PATH/.env before a local .env, and to convert several fatal teardown errors into warnings/skips (missing dependencies, Docker unavailability, missing neonctl / NEON_* vars), allowing teardown to continue and report warnings rather than fail.

Changes

Cohort / File(s)	Summary
Teardown script .superset/teardown.sh	Env loading now sources $SUPERSET_ROOT_PATH/.env first, then local .env. Dependency/detection failures (missing tools, Docker, neonctl, NEON_PROJECT_ID/NEON_BRANCH_ID) are downgraded to warnings/skips and return success, and summary reporting reflects warnings/skips instead of hard failures.

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant TeardownScript as Teardown
  participant EnvFiles as Env
  participant Docker
  participant NeonCtl as neonctl
  participant Summary

  User->>Teardown: run .superset/teardown.sh
  Teardown->>Env: source $SUPERSET_ROOT_PATH/.env then .env (fallback)
  Env-->>Teardown: env variables (or warning if none)
  Teardown->>Teardown: check dependencies (emit warnings, do not fail)
  Teardown->>Docker: attempt stop container
  alt Docker available
    Docker-->>Teardown: stopped
  else Docker missing/unavailable
    Docker-->>Teardown: warning / skip
  end
  Teardown->>NeonCtl: pre-check neonctl binary
  alt neonctl available and NEON vars present
    NeonCtl-->>Teardown: delete branch (or warn if not found)
  else neonctl missing or NEON vars absent
    NeonCtl-->>Teardown: warning / skip
  end
  Teardown->>Summary: append warnings/skips and finish (exit 0)
  Summary-->>User: final teardown summary (warnings, skipped steps)

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• Fix neon setup/teardown scripts #223: Modifies .superset/teardown.sh with overlapping changes to env sourcing and Neon branch handling.
• fix: make teardown resilient to already-deleted Neon branches #1289: Makes Neon branch deletion tolerant of missing NEON_BRANCH_ID and branch-not-found cases.
• Fix teardown to work when no .env passed and no branch #1292: Similar relaxations to env sourcing and non-fatal/skipped Neon branch deletion behavior.

Poem

🐰 I hopped to the root to fetch the env with care,
I swapped sharp errors for soft warnings in the air,
If neonctl's gone, I tiptoe and skip,
Containers may linger — I still close the trip,
Tidy trails behind me, with a carrot and a flair. 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately captures the main objective: making the teardown script resilient to missing dependencies and environment variables.
Description check	✅ Passed	The description covers all template sections: summary of changes, related issues (closes #1394), type of change context, and a test plan. All required information is present.
Linked Issues check	✅ Passed	The pull request fulfills all coding objectives from #1394: sources root .env via SUPERSET_ROOT_PATH, downgrades missing deps to warnings, skips Neon branch deletion gracefully, and tracks skipped steps in summary.
Out of Scope Changes check	✅ Passed	All changes in teardown.sh directly address the objectives from #1394; no out-of-scope modifications are present.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments

.superset/teardown.sh (1)

190-196: step_load_env now always returns 0, making the failure guard on line 191 dead code.

Since step_load_env returns 0 in all paths (success or skip), the step_failed "Load environment variables" on line 192 is unreachable. Not harmful — it's defensive — but it could be misleading to a future reader who expects the env step can actually fail. Consider aligning it with the step_check_dependencies pattern (line 196) for consistency, or keep it as-is for safety if you anticipate re-introducing a failure path later.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In @.superset/teardown.sh:
- Around line 140-143: The checks that bail out when prerequisites are missing
should mark the step as skipped before returning: in the block that checks for
neonctl (the if ! command -v neonctl ...), and in the checks for NEON_PROJECT_ID
and NEON_BRANCH_ID, call step_skipped "neon (reason)" (or similar descriptive
text) immediately before return 0 so the summary records the skip; apply the
same change in step_stop_electric where Docker is missing (call step_skipped
"electric (docker missing)" then return 0). Ensure you use the exact
step_skipped function and provide a concise reason string for each early-exit
path.
- Around line 75-78: The current check only tests if SUPERSET_ROOT_PATH is empty
and never verifies that $SUPERSET_ROOT_PATH/.env actually exists before claiming
"Environment variables loaded"; update the logic in the script to track whether
any .env was sourced: explicitly test for the existence of
"$SUPERSET_ROOT_PATH/.env" when SUPERSET_ROOT_PATH is non-empty and source it if
present, otherwise fall back to sourcing the local ".env" if it exists, and if
neither file was sourced call error and return 1 (keep the existing error and
return behavior). Ensure the script sets or checks a boolean/flag (e.g.,
sourced_any) so the final "Environment variables loaded" message is only printed
when at least one .env file was successfully sourced.