fix(install_dkim_records): guard sqlite insert, fix if-grep, remove dead code#15
Draft
troglodyne wants to merge 1 commit into
Draft
fix(install_dkim_records): guard sqlite insert, fix if-grep, remove dead code#15troglodyne wants to merge 1 commit into
troglodyne wants to merge 1 commit into
Conversation
…ead code Three bugs in the script: 1. Duplicate DKIM records on redeploy: plain INSERT fails silently on the second run because the record already exists. Switch to INSERT OR REPLACE. 2. Broken registrar update check: `if [ echo "$X" | grep "..." ]` does not work — the pipe runs inside `[` which is a test expression, not a subshell. The condition was always true, always attempting an UPDATE instead of checking first. Replaced with `echo "$X" | grep -q "..."`. 3. Dead code: the block after `#TODO disabling for now. exit 0;` was unreachable. Merged the correct implementation from the dead block into the active section and dropped the duplicate. Also guards the registrar update with `[ -x /opt/lexicon/$1 ]` so deployments without lexicon configured skip the registrar step cleanly instead of failing. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This was referenced Apr 25, 2026
troglodyne
commented
Apr 26, 2026
| fi | ||
|
|
||
| #TODO disabling for now. | ||
| exit 0; |
Contributor
Author
There was a problem hiding this comment.
Behavior change. Probably will need testing
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.
What
Three bug fixes in
scripts/install_dkim_records.Why
Bug 1 — duplicate SQLite records on redeploy: The DKIM TXT records were inserted with plain
INSERT, which silently creates duplicates (or fails) on every subsequent provisioning run. Changed toINSERT OR REPLACE.Bug 2 — broken registrar update check: The condition
if [ echo "$X" | grep "..." ]doesn't work as intended. Inside[ ]the shell does not treat|as a pipe, so the grep never ran; the condition was always true and always attempted an UPDATE regardless of whether the record existed. Fixed toif echo "$X" | grep -q "...".Bug 3 — dead code: The block after
#TODO disabling for now. exit 0;was unreachable. The dead block contained a correct version of the registrar logic (bothmail._domainkeyanddefault._domainkeyselectors). Merged it into the active section and removed the duplicate.How
INSERT OR REPLACEhandles the dedupecho "$X" | grep -q "pattern"is the correct idiom for checking pipeline output in anif[ -x "/opt/lexicon/$1" ] || exit 0guards the registrar update so deployments without lexicon configured skip it cleanlymail._domainkeyanddefault._domainkey) now handled in the active code pathTesting
bash -npasses. Logic verified by manual inspection against the dead-code block's intent.🤖 Generated with Claude Code