fix(reconcile): distinguish 'removed from Airbyte' from 'not installed here' - #2023
Merged
cyberantonz merged 4 commits intoJul 29, 2026
Conversation
📝 WalkthroughWalkthroughCascade deletion now distinguishes actual resource removal from missing-resource no-ops, adjusts WARN/INFO logging, and classifies affected connectors as changed or skipped. ChangesCascade delete reconciliation
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Contributor
|
This was referenced Jul 29, 2026
mitasovr
approved these changes
Jul 29, 2026
cyberantonz
enabled auto-merge (squash)
July 29, 2026 12:12
…d here' reconcile_cascade_delete logged 'Secret was deleted in Kubernetes — removed connector from Airbyte' for EVERY descriptor without a matching Secret — including connectors never configured on the cluster, where nothing existed and nothing was removed (observed on vz: claude-team WARN with no Secret and no source ever present). The loop is stateless per tick and cannot know whether a Secret 'was deleted'; what it can know is whether the cascade actually removed anything. Now: count deleted sources + detect CronWorkflow removal (kubectl --ignore-not-found prints output only when the object existed) and branch — WARN 'Secret missing — removed N source(s) + CronWorkflow' when something was removed; INFO 'not installed on this cluster; nothing to remove' otherwise. The caller counts the empty case as SKIPPED instead of CHANGED, so uninstalled descriptors stop inflating the changed-count every tick. Spec flow inst-cd-log updated to match. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Anton Zelenov <antonz@constructor.tech>
cyberantonz
force-pushed
the
fix/reconcile-cascade-not-installed
branch
from
July 29, 2026 12:18
9968ab4 to
200cf65
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/ingestion/reconcile-connectors/lib/reconcile.sh`:
- Around line 157-164: Update the dry-run message documented in FEATURE.md for
the reconcile cascade-delete behavior to match the intentional output from the
_RECONCILE_CASCADE_REMOVED dry-run branch: “would remove ${connector} from
Airbyte — no Secret in Kubernetes”. Preserve the surrounding feature
specification and document the updated wording wherever the old “would
cascade-delete ${connector}: secret missing” message appears.
- Around line 183-187: Update the deletion loop around ab_delete_source so
removed_sources is incremented only when the deletion command succeeds. Stop
suppressing deletion errors with unconditional || true or discarded stderr;
preserve and report failures, including those from argo_delete_cronworkflow,
rather than treating failed deletions as already absent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: cc818771-0af6-4aa6-86ce-c18f1d6d02fc
📒 Files selected for processing (2)
docs/components/airbyte-toolkit/specs/feature-reconcile/FEATURE.mdsrc/ingestion/reconcile-connectors/lib/reconcile.sh
Comment on lines
+157
to
+164
| # Set to 1 when this call actually removed something (sources and/or the | ||
| # CronWorkflow); the caller counts CHANGED vs SKIPPED off it. Dry-run | ||
| # reports 1 — it would attempt the removal. | ||
| _RECONCILE_CASCADE_REMOVED=0 | ||
| if [[ "${RECONCILE_DRY_RUN:-0}" -eq 1 ]]; then # RULE-DEFAULTS-OK: feature flag — OFF when caller doesn't opt in | ||
| # @cpt-begin:cpt-insightspec-algo-reconcile-cascade-delete-cronworkflow:p1:inst-cd-dry-run-guard | ||
| log_line WARN "would remove ${connector} from Airbyte — its Secret was deleted in Kubernetes" | ||
| _RECONCILE_CASCADE_REMOVED=1 | ||
| log_line WARN "would remove ${connector} from Airbyte — no Secret in Kubernetes" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Update the documented dry-run message.
Line 164 now emits would remove ${connector} from Airbyte — no Secret in Kubernetes, while docs/components/airbyte-toolkit/specs/feature-reconcile/FEATURE.md Line 256 still documents would cascade-delete ${connector}: secret missing. Keep the feature specification aligned with the intentional message change.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/ingestion/reconcile-connectors/lib/reconcile.sh` around lines 157 - 164,
Update the dry-run message documented in FEATURE.md for the reconcile
cascade-delete behavior to match the intentional output from the
_RECONCILE_CASCADE_REMOVED dry-run branch: “would remove ${connector} from
Airbyte — no Secret in Kubernetes”. Preserve the surrounding feature
specification and document the updated wording wherever the old “would
cascade-delete ${connector}: secret missing” message appears.
Comment on lines
+183
to
+187
| local removed_sources=0 | ||
| while IFS= read -r conn_id; do | ||
| [[ -n "${conn_id}" ]] || continue | ||
| ab_delete_source "${conn_id}" >/dev/null 2>&1 || true | ||
| removed_sources=$((removed_sources + 1)) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file excerpt =="
sed -n '150,235p' src/ingestion/reconcile-connectors/lib/reconcile.sh
echo
echo "== relevant function definitions/usages =="
rg -n "ab_delete_source|argo_delete_cronworkflow|removed_sources|skipped|nothing to remove|for.*conn_id|removed_sources" src/ingestion/reconcile-connectors/lib/reconcile.sh
echo
echo "== outlines =="
ast-grep outline src/ingestion/reconcile-connectors/lib/reconcile.sh || trueRepository: constructorfabric/insight
Length of output: 5197
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate ab_delete_source/argo_delete_cronworkflow definitions and call sites =="
rg -n "function .*ab_delete_source|ab_delete_source\(|function .*argo_delete_cronworkflow|argo_delete_cronworkflow\(" src/ingestion/reconcile-connectors/lib/reconcile.sh
echo
echo "== function definitions with context =="
# Print roughly 25-40 lines around each definition for the implementation shape.
rg -C 25 "function .*ab_delete_source|function .*argo_delete_cronworkflow" src/ingestion/reconcile-connectors/lib/reconcile.sh || true
echo
echo "== all references in reconcile script =="
rg -n "remove|failed|delete|exit|return|ab_delete_source|argo_delete_cronworkflow|removed_sources|removed_what|nothing to remove|not installed" src/ingestion/reconcile-connectors/lib/reconcile.shRepository: constructorfabric/insight
Length of output: 245
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate files with delete helper definitions/usages =="
fd -e sh -e py . src/ingestion/reconcile-connectors 2>/dev/null | while read -r f; do
echo "--- $f"
rg -n "ab_delete_source|argo_delete_cronworkflow" "$f" || true
done
echo
echo "== top-level file list snippet relevant to reconcile connector =="
git ls-files src/ingestion/reconcile-connectors | sed -n '1,120p'Repository: constructorfabric/insight
Length of output: 6698
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== airbyte helper definitions =="
sed -n '530,625p' src/ingestion/reconcile-connectors/lib/airbyte.sh
echo
echo "== argo helper definitions =="
sed -n '1,95p' src/ingestion/reconcile-connectors/lib/argo.sh
echo
echo "== reconciliation include/source references =="
rg -n "airbyte\.sh|argo\.sh|source .*\.sh|\. .*\.sh" src/ingestion/reconcile-connectors/lib src/ingestion/reconcile-connectors/main.sh | sed -n '1,160p'Repository: constructorfabric/insight
Length of output: 12377
Do not count deletion failures as removed.
removed_sources is incremented after ab_delete_source ... || true, so failed Airbyte source deletes are treated as successful removals. Also, wrapping argo_delete_cronworkflow with 2>/dev/null || true discards its failure output/error and makes the cascade believe the connector was already absent. Increment counters only when deletion actually succeeds and handle/report failures instead of converting them into “not installed / nothing to remove”.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/ingestion/reconcile-connectors/lib/reconcile.sh` around lines 183 - 187,
Update the deletion loop around ab_delete_source so removed_sources is
incremented only when the deletion command succeeds. Stop suppressing deletion
errors with unconditional || true or discarded stderr; preserve and report
failures, including those from argo_delete_cronworkflow, rather than treating
failed deletions as already absent.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Observed on the vz warmup:
{"level":"warn","msg":"claude-team: Secret was deleted in Kubernetes — removed connector from Airbyte"}— while no claude-team Secret (or Airbyte source) ever existed on that cluster.reconcile_cascade_deletelogged that WARN unconditionally for every descriptor without a matching Secret and counted it as CHANGED, every tick.Change
kubectl delete --ignore-not-found's output (non-empty only when the object existed)."<name>: Secret missing in Kubernetes — removed N source(s) + CronWorkflow"."<name>: no Secret and no Airbyte/Argo resources — not installed on this cluster; nothing to remove", counted as SKIPPED instead of CHANGED (previously every not-configured connector inflated the changed-count on every 15-min tick).feature-reconcile/FEATURE.mdflowcascade-delete-on-secret-missingstepinst-cd-lognow describes the branch.cfs validate --artifact … --skip-code --local-only: the one reported error (ref-no-definitionforcpt-insightspec-algo-reconcile-classify-bump, line 913) is pre-existing on main — verified by validating with this change stashed.🤖 Generated with Claude Code
Summary by CodeRabbit